Skip to content

Commit

Permalink
merge
Browse files Browse the repository at this point in the history
  • Loading branch information
aspicer committed Jun 18, 2024
2 parents e9b4964 + 6a28f70 commit 3850eb4
Show file tree
Hide file tree
Showing 31 changed files with 188 additions and 131 deletions.
4 changes: 4 additions & 0 deletions .devcontainer/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# hadolint global ignore=DL3004

# hadolint doesn't like changes to this file, but it is only used for local dev

# Defines the environment you're dropped into with codespaces
# I've take
# https://github.com/microsoft/vscode-dev-containers/blob/main/containers/python-3/.devcontainer/Dockerfile
Expand Down
61 changes: 31 additions & 30 deletions .github/workflows/ci-backend.yml
Original file line number Diff line number Diff line change
Expand Up @@ -177,22 +177,21 @@ jobs:
sudo apt-get install libxml2-dev libxmlsec1-dev libxmlsec1-openssl
# First running migrations from master, to simulate the real-world scenario

- name: Checkout master
uses: actions/checkout@v3
with:
ref: master

- name: Install python dependencies for master
run: |
uv pip install --system -r requirements.txt -r requirements-dev.txt
- name: Run migrations up to master
run: |
python manage.py migrate
# Commented out to move to Python 3.11. Uncomment after deploy.
# - name: Checkout master
# uses: actions/checkout@v3
# with:
# ref: master
#
# - name: Install python dependencies for master
# run: |
# uv pip install --system -r requirements.txt -r requirements-dev.txt
#
# - name: Run migrations up to master
# run: |
# python manage.py migrate

# Now we can consider this PR's migrations

- name: Checkout this PR
uses: actions/checkout@v3

Expand All @@ -204,22 +203,24 @@ jobs:
run: |
python manage.py migrate
- name: Check migrations
run: |
python manage.py makemigrations --check --dry-run
git fetch origin master
# `git diff --name-only` returns a list of files that were changed - added OR deleted OR modified
# With `--name-status` we get the same, but including a column for status, respectively: A, D, M
# In this check we exclusively care about files that were
# added (A) in posthog/migrations/. We also want to ignore
# initial migrations (0001_*) as these are guaranteed to be
# run on initial setup where there is no data.
git diff --name-status origin/master..HEAD | grep "A\sposthog/migrations/" | awk '{print $2}' | grep -v migrations/0001_ | python manage.py test_migrations_are_safe
- name: Check CH migrations
run: |
# Same as above, except now for CH looking at files that were added in posthog/clickhouse/migrations/
git diff --name-status origin/master..HEAD | grep "A\sposthog/clickhouse/migrations/" | awk '{print $2}' | python manage.py test_ch_migrations_are_safe
# Commented out to move to Python 3.11. Uncomment after deploy.
#
# - name: Check migrations
# run: |
# python manage.py makemigrations --check --dry-run
# git fetch origin master
# # `git diff --name-only` returns a list of files that were changed - added OR deleted OR modified
# # With `--name-status` we get the same, but including a column for status, respectively: A, D, M
# # In this check we exclusively care about files that were
# # added (A) in posthog/migrations/. We also want to ignore
# # initial migrations (0001_*) as these are guaranteed to be
# # run on initial setup where there is no data.
# git diff --name-status origin/master..HEAD | grep "A\sposthog/migrations/" | awk '{print $2}' | grep -v migrations/0001_ | python manage.py test_migrations_are_safe
#
# - name: Check CH migrations
# run: |
# # Same as above, except now for CH looking at files that were added in posthog/clickhouse/migrations/
# git diff --name-status origin/master..HEAD | grep "A\sposthog/clickhouse/migrations/" | awk '{print $2}' | python manage.py test_ch_migrations_are_safe

django:
needs: changes
Expand Down
8 changes: 7 additions & 1 deletion bin/build-schema-python.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ set -e

# Generate schema.py from schema.json
datamodel-codegen \
--class-name='SchemaRoot' --collapse-root-models --target-python-version 3.10 --disable-timestamp \
--class-name='SchemaRoot' --collapse-root-models --target-python-version 3.11 --disable-timestamp \
--use-one-literal-as-default --use-default --use-default-kwarg --use-subclass-enum \
--input frontend/src/queries/schema.json --input-file-type jsonschema \
--output posthog/schema.py --output-model-type pydantic_v2.BaseModel \
Expand All @@ -29,3 +29,9 @@ if [[ "$OSTYPE" == "darwin"* ]]; then
else
sed -i -e 's/Optional\[PropertyOperator\] = \("[A-Za-z_]*"\)/Optional[PropertyOperator] = PropertyOperator(\1)/g' posthog/schema.py
fi

# Replace class Foo(str, Enum) with class Foo(StrEnum) for proper handling in format strings in python 3.11
# Remove this when https://github.com/koxudaxi/datamodel-code-generator/issues/1313 is resolved

sed -i -e 's/str, Enum/StrEnum/g' posthog/schema.py
sed -i 's/from enum import Enum/from enum import Enum, StrEnum/g' posthog/schema.py
2 changes: 1 addition & 1 deletion ee/api/test/__snapshots__/test_time_to_see_data.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"first_name": "",
"last_name": "",
"email": "",
"is_email_verified": false
"is_email_verified": null
}
},
"children": [
Expand Down
2 changes: 2 additions & 0 deletions posthog/api/comments.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@

from posthog.api.routing import TeamAndOrgViewSetMixin
from posthog.api.shared import UserBasicSerializer
from posthog.api.utils import ClassicBehaviorBooleanFieldSerializer
from posthog.models.comment import Comment


class CommentSerializer(serializers.ModelSerializer):
created_by = UserBasicSerializer(read_only=True)
deleted = ClassicBehaviorBooleanFieldSerializer()

class Meta:
model = Comment
Expand Down
4 changes: 4 additions & 0 deletions posthog/api/feature_flag.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from posthog.api.shared import UserBasicSerializer
from posthog.api.tagged_item import TaggedItemSerializerMixin, TaggedItemViewSetMixin
from posthog.api.dashboards.dashboard import Dashboard
from posthog.api.utils import ClassicBehaviorBooleanFieldSerializer
from posthog.auth import PersonalAPIKeyAuthentication, TemporaryTokenAuthentication
from posthog.constants import FlagRequestType
from posthog.event_usage import report_user_action
Expand Down Expand Up @@ -89,6 +90,9 @@ class FeatureFlagSerializer(TaggedItemSerializerMixin, serializers.HyperlinkedMo
is_simple_flag = serializers.SerializerMethodField()
rollout_percentage = serializers.SerializerMethodField()

ensure_experience_continuity = ClassicBehaviorBooleanFieldSerializer()
has_enriched_analytics = ClassicBehaviorBooleanFieldSerializer()

experiment_set: serializers.PrimaryKeyRelatedField = serializers.PrimaryKeyRelatedField(many=True, read_only=True)
surveys: serializers.SerializerMethodField = serializers.SerializerMethodField()
features: serializers.SerializerMethodField = serializers.SerializerMethodField()
Expand Down
3 changes: 3 additions & 0 deletions posthog/api/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

from posthog.api.routing import TeamAndOrgViewSetMixin
from posthog.api.shared import FiltersSerializer
from posthog.api.utils import ClassicBehaviorBooleanFieldSerializer
from posthog.models import Plugin, PluginAttachment, PluginConfig, User
from posthog.models.activity_logging.activity_log import (
ActivityPage,
Expand Down Expand Up @@ -586,6 +587,8 @@ class PluginConfigSerializer(serializers.ModelSerializer):
delivery_rate_24h = serializers.SerializerMethodField()
error = serializers.SerializerMethodField()

deleted = ClassicBehaviorBooleanFieldSerializer()

class Meta:
model = PluginConfig
fields = [
Expand Down
26 changes: 26 additions & 0 deletions posthog/api/routing.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,32 @@
class DefaultRouterPlusPlus(ExtendedDefaultRouter):
"""DefaultRouter with optional trailing slash and drf-extensions nesting."""

# This is an override because of changes in djangorestframework 3.15, which is required for python 3.11
# changes taken from and explained here: https://github.com/nautobot/nautobot/pull/5546/files#diff-81850a2ccad5814aab4f477d447f85cc0a82e9c10fd88fd72327cda51a750471R30
def _register(self, prefix, viewset, basename=None):
"""
Override DRF's BaseRouter.register() to bypass an unnecessary restriction added in version 3.15.0.
(Reference: https://github.com/encode/django-rest-framework/pull/8438)
"""
if basename is None:
basename = self.get_default_basename(viewset)

# DRF:
# if self.is_already_registered(basename):
# msg = (f'Router with basename "{basename}" is already registered. '
# f'Please provide a unique basename for viewset "{viewset}"')
# raise ImproperlyConfigured(msg)
#
# We bypass this because we have at least one use case (/api/extras/jobs/) where we are *intentionally*
# registering two viewsets with the same basename, but have carefully defined them so as not to conflict.

# resuming standard DRF code...
self.registry.append((prefix, viewset, basename))

# invalidate the urls cache
if hasattr(self, "_urls"):
del self._urls

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.trailing_slash = r"/?"
Expand Down
2 changes: 1 addition & 1 deletion posthog/api/test/__snapshots__/test_api_docs.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
"/home/runner/work/posthog/posthog/posthog/api/property_definition.py: Error [PropertyDefinitionViewSet]: exception raised while getting serializer. Hint: Is get_serializer_class() returning None or is get_queryset() not working without a request? Ignoring the view for now. (Exception: 'AnonymousUser' object has no attribute 'organization')",
'/home/runner/work/posthog/posthog/posthog/api/property_definition.py: Warning [PropertyDefinitionViewSet]: could not derive type of path parameter "project_id" because model "posthog.models.property_definition.PropertyDefinition" contained no such field. Consider annotating parameter with @extend_schema. Defaulting to "string".',
'/home/runner/work/posthog/posthog/posthog/api/query.py: Warning [QueryViewSet]: could not derive type of path parameter "project_id" because it is untyped and obtaining queryset from the viewset failed. Consider adding a type to the path (e.g. <int:project_id>) or annotating the parameter type with @extend_schema. Defaulting to "string".',
'/opt/hostedtoolcache/Python/3.10.10/x64/lib/python3.10/site-packages/pydantic/_internal/_model_construction.py: Warning [QueryViewSet > ModelMetaclass]: Encountered 2 components with identical names "Person" and different classes <class \'str\'> and <class \'posthog.api.person.PersonSerializer\'>. This will very likely result in an incorrect schema. Try renaming one.',
'/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/pydantic/_internal/_model_construction.py: Warning [QueryViewSet > ModelMetaclass]: Encountered 2 components with identical names "Person" and different classes <class \'str\'> and <class \'posthog.api.person.PersonSerializer\'>. This will very likely result in an incorrect schema. Try renaming one.',
'/home/runner/work/posthog/posthog/posthog/api/query.py: Warning [QueryViewSet]: could not derive type of path parameter "id" because it is untyped and obtaining queryset from the viewset failed. Consider adding a type to the path (e.g. <int:id>) or annotating the parameter type with @extend_schema. Defaulting to "string".',
'/home/runner/work/posthog/posthog/posthog/api/query.py: Error [QueryViewSet]: unable to guess serializer. This is graceful fallback handling for APIViews. Consider using GenericAPIView as view base class, if view is under your control. Either way you may want to add a serializer_class (or method). Ignoring view for now.',
'/home/runner/work/posthog/posthog/ee/session_recordings/session_recording_playlist.py: Warning [SessionRecordingPlaylistViewSet]: could not derive type of path parameter "project_id" because model "posthog.session_recordings.models.session_recording_playlist.SessionRecordingPlaylist" contained no such field. Consider annotating parameter with @extend_schema. Defaulting to "string".',
Expand Down
7 changes: 6 additions & 1 deletion posthog/api/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@
from posthog.api.email_verification import EmailVerifier
from posthog.api.organization import OrganizationSerializer
from posthog.api.shared import OrganizationBasicSerializer, TeamBasicSerializer
from posthog.api.utils import PublicIPOnlyHttpAdapter, raise_if_user_provided_url_unsafe
from posthog.api.utils import (
PublicIPOnlyHttpAdapter,
raise_if_user_provided_url_unsafe,
ClassicBehaviorBooleanFieldSerializer,
)
from posthog.auth import (
PersonalAPIKeyAuthentication,
SessionAuthentication,
Expand Down Expand Up @@ -84,6 +88,7 @@ class UserSerializer(serializers.ModelSerializer):
current_password = serializers.CharField(write_only=True, required=False)
notification_settings = serializers.DictField(required=False)
scene_personalisation = ScenePersonalisationBasicSerializer(many=True, read_only=True)
anonymize_data = ClassicBehaviorBooleanFieldSerializer()

class Meta:
model = User
Expand Down
12 changes: 11 additions & 1 deletion posthog/api/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,16 @@
from ipaddress import ip_address
from requests.adapters import HTTPAdapter
from typing import Literal, Optional, Union

from rest_framework.fields import Field
from urllib3 import HTTPSConnectionPool, HTTPConnectionPool, PoolManager
from uuid import UUID

import structlog
from django.core.exceptions import RequestDataTooBig
from django.db.models import QuerySet
from prometheus_client import Counter
from rest_framework import request, status
from rest_framework import request, status, serializers
from rest_framework.exceptions import ValidationError
from statshog.defaults.django import statsd

Expand All @@ -34,6 +36,14 @@ class PaginationMode(Enum):
previous = auto()


# This overrides a change in DRF 3.15 that alters our behavior. If the user passes an empty argument,
# the new version keeps it as null vs coalescing it to the default.
# Don't add this to new classes
class ClassicBehaviorBooleanFieldSerializer(serializers.BooleanField):
def __init__(self, **kwargs):
Field.__init__(self, allow_null=True, required=False, **kwargs)


def get_target_entity(filter: Union[Filter, StickinessFilter]) -> Entity:
# Except for "events", we require an entity id and type to be provided
if not filter.target_entity_id and filter.target_entity_type != "events":
Expand Down
2 changes: 1 addition & 1 deletion posthog/batch_exports/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ def interval_time_delta(self) -> timedelta:
raise ValueError(f"Invalid interval: '{self.interval}'")


class BatchExportLogEntryLevel(str, enum.Enum):
class BatchExportLogEntryLevel(enum.StrEnum):
"""Enumeration of batch export log levels."""

DEBUG = "DEBUG"
Expand Down
4 changes: 2 additions & 2 deletions posthog/clickhouse/table_engines.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import uuid
from enum import Enum
from enum import StrEnum
from typing import Optional

from django.conf import settings


class ReplicationScheme(str, Enum):
class ReplicationScheme(StrEnum):
NOT_SHARDED = "NOT_SHARDED"
SHARDED = "SHARDED"
REPLICATED = "REPLICATED"
Expand Down
26 changes: 13 additions & 13 deletions posthog/constants.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from enum import Enum
from enum import StrEnum
from typing import Literal

from semantic_version import Version
Expand All @@ -9,7 +9,7 @@

# N.B. Keep this in sync with frontend enum (types.ts)
# AND ensure it is added to the Billing Service
class AvailableFeature(str, Enum):
class AvailableFeature(StrEnum):
ZAPIER = "zapier"
ORGANIZATIONS_PROJECTS = "organizations_projects"
PROJECT_BASED_PERMISSIONING = "project_based_permissioning"
Expand Down Expand Up @@ -215,19 +215,19 @@ class AvailableFeature(str, Enum):
BREAKDOWN_TYPES = Literal["event", "person", "cohort", "group", "session", "hogql"]


class FunnelOrderType(str, Enum):
class FunnelOrderType(StrEnum):
STRICT = "strict"
UNORDERED = "unordered"
ORDERED = "ordered"


class FunnelVizType(str, Enum):
class FunnelVizType(StrEnum):
TRENDS = "trends"
TIME_TO_CONVERT = "time_to_convert"
STEPS = "steps"


class FunnelCorrelationType(str, Enum):
class FunnelCorrelationType(StrEnum):
EVENTS = "events"
PROPERTIES = "properties"
EVENT_WITH_PROPERTIES = "event_with_properties"
Expand All @@ -240,7 +240,7 @@ class FunnelCorrelationType(str, Enum):
PERSON_UUID_FILTER = "person_uuid"


class AnalyticsDBMS(str, Enum):
class AnalyticsDBMS(StrEnum):
POSTGRES = "postgres"
CLICKHOUSE = "clickhouse"

Expand All @@ -251,34 +251,34 @@ class AnalyticsDBMS(str, Enum):
MONTHLY_ACTIVE = "monthly_active"


class RetentionQueryType(str, Enum):
class RetentionQueryType(StrEnum):
RETURNING = "returning"
TARGET = "target"
TARGET_FIRST_TIME = "target_first_time"


class ExperimentSignificanceCode(str, Enum):
class ExperimentSignificanceCode(StrEnum):
SIGNIFICANT = "significant"
NOT_ENOUGH_EXPOSURE = "not_enough_exposure"
LOW_WIN_PROBABILITY = "low_win_probability"
HIGH_LOSS = "high_loss"
HIGH_P_VALUE = "high_p_value"


class ExperimentNoResultsErrorKeys(str, Enum):
class ExperimentNoResultsErrorKeys(StrEnum):
NO_EVENTS = "no-events"
NO_FLAG_INFO = "no-flag-info"
NO_CONTROL_VARIANT = "no-control-variant"
NO_TEST_VARIANT = "no-test-variant"
NO_RESULTS = "no-results"


class PropertyOperatorType(str, Enum):
class PropertyOperatorType(StrEnum):
AND = "AND"
OR = "OR"


class BreakdownAttributionType(str, Enum):
class BreakdownAttributionType(StrEnum):
FIRST_TOUCH = "first_touch"
# FIRST_TOUCH attribution means the breakdown value is the first property value found within all funnel steps
LAST_TOUCH = "last_touch"
Expand All @@ -294,7 +294,7 @@ class BreakdownAttributionType(str, Enum):
GROUP_TYPES_LIMIT = 5


class EventDefinitionType(str, Enum):
class EventDefinitionType(StrEnum):
# Mimics EventDefinitionType in frontend/src/types.ts
ALL = "all"
ACTION_EVENT = "action_event"
Expand All @@ -303,7 +303,7 @@ class EventDefinitionType(str, Enum):
EVENT_CUSTOM = "event_custom"


class FlagRequestType(str, Enum):
class FlagRequestType(StrEnum):
DECIDE = "decide"
LOCAL_EVALUATION = "local-evaluation"

Expand Down
Loading

0 comments on commit 3850eb4

Please sign in to comment.