Skip to content

Conversation

webjunkie
Copy link
Contributor

@webjunkie webjunkie commented Sep 15, 2025

Problem

We are on Python 3.11 still

Changes

Upgrades Python interpreter from 3.11 to 3.12.11 and updates configuration to explicitly pin the patch version (==3.12.11).

Reasoning:

  • Ensures all CI jobs and developers using e.g. flox run against the same interpreter build, avoiding subtle ABI differences and dependency resolution mismatches across patch releases.
  • Aligns with our approach of pinning application dependencies — the Python runtime should be just as reproducible.

How did you test this code?

Relying on tests
Checked compatibility with various packages

@webjunkie webjunkie force-pushed the chore/upgrade-to-python-3.12.11 branch from a5ae229 to 9d503bc Compare September 16, 2025 07:57
Copy link
Contributor

github-actions bot commented Sep 17, 2025

Size Change: 0 B

Total Size: 3.05 MB

ℹ️ View Unchanged
Filename Size
frontend/dist/toolbar.js 3.05 MB

compressed-size-action

@webjunkie
Copy link
Contributor Author

@pauldambra and @aspicer I guess you were involved in the last Python updated. Any particular area to pay special attention to?

@pauldambra
Copy link
Member

i think that the django upgrades have been more painful than the python ones

my guess is saml / sso / openssl is the one to watch

i bet we're out of date on dependencies and type packages and that's where any changes will come from

god speed!

@webjunkie
Copy link
Contributor Author

i bet we're out of date on dependencies and type packages and that's where any changes will come from

So far it's looking suspiciously good 🫠

@webjunkie webjunkie self-assigned this Sep 18, 2025
@webjunkie webjunkie marked this pull request as ready for review September 18, 2025 10:08
@posthog-bot posthog-bot requested review from a team September 18, 2025 10:10
@posthog-project-board-bot posthog-project-board-bot bot moved this to In Review in Feature Flags Sep 18, 2025
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

59 files reviewed, 2 comments

Edit Code Review Bot Settings | Greptile

Copy link
Contributor

github-actions bot commented Oct 6, 2025

⚠️ Flapping Screenshots Detected

The following screenshots have been updated multiple times by the bot, indicating test instability:

  • playwright/__snapshots__/pageview-trends-insight.png

What this means:

  • These screenshots are changing on each test run (timing issues, rendering differences, etc.)
  • This prevents reliable verification and blocks auto-merge
  • Human intervention is required

How to fix:

  1. Investigate flakiness: Check test timing, wait for elements, stabilize animations
  2. Fix the underlying issue: Don't just update snapshots repeatedly
  3. Verify locally: Run tests multiple times to ensure stability
  4. Push your fix: This will reset the flap counter

If you need to proceed anyway:

  • Fix the test flakiness first (recommended)
  • Or manually verify snapshots are acceptable and revert the bot commits, then push a fix

The workflow has been failed to prevent merging unstable tests.

webjunkie and others added 4 commits October 6, 2025 17:22
…nd SQL modules

Fixes E2E test failures where ClickHouse distributed tables reference wrong database name ('default' instead of 'posthog_test').

Root cause: Django settings imported at module level bind to values before test fixtures set environment variables.

Changes:
- table_engines.py: Remove default parameter binding in Distributed.__init__, evaluate cluster/database at call time
- person/sql.py: Switch from posthog.settings to django.conf.settings, convert module-level SQL to functions
- error_tracking/sql.py: Convert remaining module-level SQL strings to functions
- Migrations and test files: Add () to call converted SQL functions

Technical detail: django.conf.settings is a LazySettings proxy that defers attribute access until call time, preventing early binding.
…odule import

Changed from `from posthog import settings` to `from django.conf import settings`
to align with master's pattern and use Django's LazySettings proxy.

Django's settings proxy defers ALL attribute lookups until access time, making
it immune to import timing issues in both Python 3.11 and 3.12.

This is the correct, battle-tested Django pattern that master already uses.

Files updated:
- posthog/models/app_metrics2/sql.py
- posthog/models/person/sql.py
- posthog/models/error_tracking/sql.py
- posthog/clickhouse/table_engines.py
- posthog/hogql/printer.py
Copy link
Contributor

github-actions bot commented Oct 6, 2025

⚠️ Flapping Screenshots Detected

The following screenshots have been updated multiple times by the bot, indicating test instability:

  • playwright/__snapshots__/pageview-trends-insight.png

What this means:

  • These screenshots are changing on each test run (timing issues, rendering differences, etc.)
  • This prevents reliable verification and blocks auto-merge
  • Human intervention is required

How to fix:

  1. Investigate flakiness: Check test timing, wait for elements, stabilize animations
  2. Fix the underlying issue: Don't just update snapshots repeatedly
  3. Verify locally: Run tests multiple times to ensure stability
  4. Push your fix: This will reset the flap counter

If you need to proceed anyway:

  • Fix the test flakiness first (recommended)
  • Or manually verify snapshots are acceptable and revert the bot commits, then push a fix

The workflow has been failed to prevent merging unstable tests.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 12)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

github-actions bot and others added 4 commits October 6, 2025 20:03
APP_METRICS_MV_TABLE_SQL was a lambda with an f-string that evaluated
settings.CLICKHOUSE_DATABASE at module import time instead of call time.
This caused tests to create tables in 'default' database instead of
'posthog_test'.

Convert lambda to proper function so settings are evaluated at runtime.
# Conflicts:
#	frontend/__snapshots__/components-playerinspector-itemevent--group-identify-event--dark.png
#	frontend/__snapshots__/components-playerinspector-itemevent--group-identify-event--light.png
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 5)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 5)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

# Conflicts:
#	posthog/async_migrations/examples/example.py
#	posthog/clickhouse/table_engines.py
@webjunkie webjunkie force-pushed the chore/upgrade-to-python-3.12.11 branch from 757cabe to b408a46 Compare October 7, 2025 19:43
# Conflicts:
#	.github/workflows/ci-backend-update-test-timing.yml
#	.github/workflows/ci-backend.yml
#	.github/workflows/ci-e2e-playwright.yml
#	.github/workflows/ci-python.yml
#	posthog/api/test/__snapshots__/test_api_docs.ambr
#	uv.lock
Copy link
Contributor

Migration SQL Changes

Hey 👋, we've detected some migrations on this PR. Here's the SQL output for each migration, make sure they make sense:

posthog/migrations/0463_datawarehousemodelpath_and_more.py

BEGIN;
--
-- Raw SQL operation
--
CREATE EXTENSION IF NOT EXISTS ltree;
--
-- Create model DataWarehouseModelPath
--
CREATE TABLE "posthog_datawarehousemodelpath" ("created_at" timestamp with time zone NOT NULL, "updated_at" timestamp with time zone NULL, "id" uuid NOT NULL PRIMARY KEY, "path" ltree NOT NULL, "created_by_id" integer NULL, "saved_query_id" uuid NULL, "table_id" uuid NULL, "team_id" integer NOT NULL);
--
-- Create constraint unique_team_id_path on model datawarehousemodelpath
--
ALTER TABLE "posthog_datawarehousemodelpath" ADD CONSTRAINT "unique_team_id_path" UNIQUE ("team_id", "path") DEFERRABLE INITIALLY IMMEDIATE;
ALTER TABLE "posthog_datawarehousemodelpath" ADD CONSTRAINT "posthog_datawarehous_created_by_id_68be5ace_fk_posthog_u" FOREIGN KEY ("created_by_id") REFERENCES "posthog_user" ("id") DEFERRABLE INITIALLY DEFERRED;
ALTER TABLE "posthog_datawarehousemodelpath" ADD CONSTRAINT "posthog_datawarehous_saved_query_id_0ff43777_fk_posthog_d" FOREIGN KEY ("saved_query_id") REFERENCES "posthog_datawarehousesavedquery" ("id") DEFERRABLE INITIALLY DEFERRED;
ALTER TABLE "posthog_datawarehousemodelpath" ADD CONSTRAINT "posthog_datawarehous_table_id_f1314089_fk_posthog_d" FOREIGN KEY ("table_id") REFERENCES "posthog_datawarehousetable" ("id") DEFERRABLE INITIALLY DEFERRED;
ALTER TABLE "posthog_datawarehousemodelpath" ADD CONSTRAINT "posthog_datawarehous_team_id_30be3718_fk_posthog_t" FOREIGN KEY ("team_id") REFERENCES "posthog_team" ("id") DEFERRABLE INITIALLY DEFERRED;
CREATE INDEX "posthog_datawarehousemodelpath_created_by_id_68be5ace" ON "posthog_datawarehousemodelpath" ("created_by_id");
CREATE INDEX "posthog_datawarehousemodelpath_saved_query_id_0ff43777" ON "posthog_datawarehousemodelpath" ("saved_query_id");
CREATE INDEX "posthog_datawarehousemodelpath_table_id_f1314089" ON "posthog_datawarehousemodelpath" ("table_id");
CREATE INDEX "posthog_datawarehousemodelpath_team_id_30be3718" ON "posthog_datawarehousemodelpath" ("team_id");
CREATE INDEX "team_id_path" ON "posthog_datawarehousemodelpath" ("team_id", "path");
CREATE INDEX "team_id_saved_query_id" ON "posthog_datawarehousemodelpath" ("team_id", "saved_query_id");
CREATE INDEX "model_path_path" ON "posthog_datawarehousemodelpath" USING gist ("path");
COMMIT;

…sion

Removes IF NOT EXISTS clause from ltree extension creation that was added
during merge but turned out to be unnecessary. The migration runs normally
without it.
Removes snapshot files that no longer exist in master, likely due to test
changes or removal.
Updates Black and Dagster tool configurations to use Python 3.12:
- Black target version py311 -> py312
- Dagster python_version 3.11 -> 3.12

Ensures tool configurations match the Python 3.12 upgrade.
@webjunkie webjunkie merged commit 77e8ce5 into master Oct 13, 2025
186 of 187 checks passed
@github-project-automation github-project-automation bot moved this from In Review to Done in Feature Flags Oct 13, 2025
@webjunkie webjunkie deleted the chore/upgrade-to-python-3.12.11 branch October 13, 2025 06:32
webjunkie added a commit that referenced this pull request Oct 13, 2025
adamleithp pushed a commit that referenced this pull request Oct 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants