Skip to content

Commit

Permalink
Merge branch 'master' into hogql-hidden-aliases
Browse files Browse the repository at this point in the history
  • Loading branch information
mariusandra committed Nov 22, 2023
2 parents 82d342a + bcff0ea commit 349678c
Show file tree
Hide file tree
Showing 22 changed files with 113 additions and 22 deletions.
Binary file modified frontend/__snapshots__/lemon-ui-lemon-banner--closable.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified frontend/__snapshots__/lemon-ui-lemon-banner--dismissable.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified frontend/__snapshots__/lemon-ui-lemon-banner--error.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified frontend/__snapshots__/lemon-ui-lemon-banner--info.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified frontend/__snapshots__/lemon-ui-lemon-banner--success.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified frontend/__snapshots__/lemon-ui-lemon-banner--warning.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified frontend/__snapshots__/lemon-ui-lemon-button--as-links.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified frontend/__snapshots__/scenes-app-batchexports--create-export.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified frontend/__snapshots__/scenes-app-notebooks--notebooks-list.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified frontend/__snapshots__/scenes-app-surveys--surveys-list.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified frontend/__snapshots__/scenes-other-login--sso-error.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
21 changes: 11 additions & 10 deletions frontend/src/layout/navigation-3000/sidebars/personsAndGroups.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,16 +98,17 @@ export const personsAndGroupsSidebarLogic = kea<personsAndGroupsSidebarLogicType
groupType.name_singular || `${groupType.group_type} group`,
groupType.name_plural || `${groupType.group_type} groups`,
],
items: groups[groupType.group_type_index].results.map((group) => {
const { searchTerm } = values
const displayId = groupDisplayId(group.group_key, group.group_properties)
return {
key: group.group_key,
name: displayId,
url: urls.group(groupType.group_type_index, group.group_key),
searchMatch: findSearchTermInItemName(displayId, searchTerm),
} as BasicListItem
}),
items:
groups[groupType.group_type_index]?.results.map((group) => {
const { searchTerm } = values
const displayId = groupDisplayId(group.group_key, group.group_properties)
return {
key: group.group_key,
name: displayId,
url: urls.group(groupType.group_type_index, group.group_key),
searchMatch: findSearchTermInItemName(displayId, searchTerm),
} as BasicListItem
}) || [],
loading: groupsLoading[groupType.group_type_index],
// FIXME: Add remote
} as SidebarCategory)
Expand Down
9 changes: 5 additions & 4 deletions frontend/src/lib/lemon-ui/LemonBanner/LemonBanner.scss
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
.LemonBanner {
align-items: center;
border-radius: var(--radius);
padding: 0.5rem 0.75rem;
border: solid 1px var(--border-3000);
color: var(--primary-alt);
font-weight: 500;
display: flex;
align-items: center;
text-align: left;
font-weight: 500;
gap: 0.5rem;
min-height: 3rem;
padding: 0.5rem 0.75rem;
text-align: left;

&.LemonBanner--info {
background-color: var(--primary-alt-highlight);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
.PropertyGroupFilters {
.property-group {
background-color: var(--side);

.posthog-3000 & {
border-width: 1px;
}

padding: 0.5rem;
border-radius: 4px;
}
Expand Down
4 changes: 4 additions & 0 deletions frontend/src/scenes/feature-flags/FeatureFlag.scss
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@
}

.FeatureConditionCard {
.posthog-3000 & {
background: var(--bg-light);
}

.FeatureConditionCard--border--highlight {
border-color: var(--primary-3000);
}
Expand Down
11 changes: 7 additions & 4 deletions frontend/src/styles/vars.scss
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ $colors: (
'secondary-3000-hover-light': #cfd1c2,
'accent-3000-light': #eeefe9,
'bg-3000-light': #f3f4ef,
'bg-hover-3000-light': #f3f4ef,
'border-3000-light': #dadbd2,
'border-bold-3000-light': #c1c2b9,
'glass-bg-3000-light': #e4e5deb3,
Expand Down Expand Up @@ -157,11 +158,12 @@ $colors: (

'secondary-3000-dark': #1d1f27,
'secondary-3000-hover-dark': #575d77,
'accent-3000-dark': #232429,
'accent-3000-dark': #21242b,
'bg-3000-dark': #1d1f27,
'border-3000-dark': #4a4c52,
'bg-hover-3000-dark': #292b36,
'border-3000-dark': #35373e,
'border-bold-3000-dark': #3f4046,
'glass-bg-3000-dark': #1d1f27b3,
'glass-bg-3000-dark': #21242bb3,
'glass-border-3000-dark': var(--border-3000-dark),
'link-3000-dark': #f1a82c,

Expand Down Expand Up @@ -189,10 +191,11 @@ $colors: (
'secondary-3000-hover': var(--secondary-3000-hover),
'accent-3000': var(--accent-3000),
'bg-3000': var(--bg-3000),
'bg-hover-3000': var(--bg-hover-3000),
'border-3000': var(--border-3000),
'border-bold-3000': var(--border-bold-3000),
'glass-bg-3000': var(--glass-bg-3000),
'glass-border-3000': var(--glass-border-3000),
'glass-border-3000': var(--border-3000),
'link-3000': var(--link-3000),
// 'bg-light': var(--accent-3000),
'primary-3000-frame-bg': var(--primary-3000-frame-bg),
Expand Down
45 changes: 45 additions & 0 deletions posthog/api/test/batch_exports/test_delete.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,3 +241,48 @@ def test_deletes_are_partitioned_by_team_id(client: HttpClient):
# Make sure we can still get the export with the right user
response = get_batch_export(client, team.pk, batch_export_id)
assert response.status_code == status.HTTP_200_OK


@pytest.mark.django_db(transaction=True)
def test_delete_batch_export_even_without_underlying_schedule(client: HttpClient):
"""Test deleting a BatchExport completes even if underlying Schedule was already deleted."""
temporal = sync_connect()

destination_data = {
"type": "S3",
"config": {
"bucket_name": "my-production-s3-bucket",
"region": "us-east-1",
"prefix": "posthog-events/",
"aws_access_key_id": "abc123",
"aws_secret_access_key": "secret",
},
}
batch_export_data = {
"name": "my-production-s3-bucket-destination",
"destination": destination_data,
"interval": "hour",
}

organization = create_organization("Test Org")
team = create_team(organization)
user = create_user("[email protected]", "Test User", organization)
client.force_login(user)

with start_test_worker(temporal):
batch_export = create_batch_export_ok(client, team.pk, batch_export_data)
batch_export_id = batch_export["id"]

handle = temporal.get_schedule_handle(batch_export_id)
async_to_sync(handle.delete)()

with pytest.raises(RPCError):
describe_schedule(temporal, batch_export_id)

delete_batch_export_ok(client, team.pk, batch_export_id)

response = get_batch_export(client, team.pk, batch_export_id)
assert response.status_code == status.HTTP_404_NOT_FOUND

with pytest.raises(RPCError):
describe_schedule(temporal, batch_export_id)
22 changes: 19 additions & 3 deletions posthog/batch_exports/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from typing import Any

import posthoganalytics
import structlog
from django.db import transaction
from django.utils.timezone import now
from rest_framework import mixins, request, response, serializers, viewsets
Expand All @@ -27,6 +28,7 @@
BatchExportIdError,
BatchExportServiceError,
BatchExportServiceRPCError,
BatchExportServiceScheduleNotFound,
backfill_export,
cancel_running_batch_export_backfill,
delete_schedule,
Expand All @@ -49,6 +51,8 @@
from posthog.temporal.client import sync_connect
from posthog.utils import relative_date_parse

logger = structlog.get_logger(__name__)


def validate_date_input(date_input: Any) -> dt.datetime:
"""Parse any datetime input as a proper dt.datetime.
Expand Down Expand Up @@ -320,10 +324,22 @@ def unpause(self, request: request.Request, *args, **kwargs) -> response.Respons
return response.Response({"paused": False})

def perform_destroy(self, instance: BatchExport):
"""Perform a BatchExport destroy by clearing Temporal and Django state."""
instance.deleted = True
"""Perform a BatchExport destroy by clearing Temporal and Django state.
If the underlying Temporal Schedule doesn't exist, we ignore the error and proceed with the delete anyways.
The Schedule could have been manually deleted causing Django and Temporal to go out of sync. For whatever reason,
since we are deleting, we assume that we can recover from this state by finishing the delete operation by calling
instance.save().
"""
temporal = sync_connect()
delete_schedule(temporal, str(instance.pk))

instance.deleted = True

try:
delete_schedule(temporal, str(instance.pk))
except BatchExportServiceScheduleNotFound as e:
logger.warning("The Schedule %s could not be deleted as it was not found", e.schedule_id)

instance.save()

for backfill in BatchExportBackfill.objects.filter(batch_export=instance):
Expand Down
18 changes: 17 additions & 1 deletion posthog/batch_exports/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from dataclasses import asdict, dataclass, fields
from uuid import UUID

import temporalio
from asgiref.sync import async_to_sync
from temporalio.client import (
Client,
Expand Down Expand Up @@ -163,6 +164,14 @@ class BatchExportServiceRPCError(BatchExportServiceError):
"""Exception raised when the underlying Temporal RPC fails."""


class BatchExportServiceScheduleNotFound(BatchExportServiceRPCError):
"""Exception raised when the underlying Temporal RPC fails because a schedule was not found."""

def __init__(self, schedule_id: str):
self.schedule_id = schedule_id
super().__init__(f"The Temporal Schedule {schedule_id} was not found (maybe it was deleted?)")


def pause_batch_export(temporal: Client, batch_export_id: str, note: str | None = None) -> None:
"""Pause this BatchExport.
Expand Down Expand Up @@ -250,7 +259,14 @@ async def unpause_schedule(temporal: Client, schedule_id: str, note: str | None
async def delete_schedule(temporal: Client, schedule_id: str) -> None:
"""Delete a Temporal Schedule."""
handle = temporal.get_schedule_handle(schedule_id)
await handle.delete()

try:
await handle.delete()
except temporalio.service.RPCError as e:
if e.status == temporalio.service.RPCStatusCode.NOT_FOUND:
raise BatchExportServiceScheduleNotFound(schedule_id)
else:
raise BatchExportServiceRPCError() from e


@async_to_sync
Expand Down

0 comments on commit 349678c

Please sign in to comment.