Skip to content

fix(data-imports): stop duckgres enablement crashing on non-UUID control-plane org ids - #74082

Merged
Gilbert09 merged 2 commits into
masterfrom
posthog-code/fix-duckgres-enablement-uuid-crash
Jul 28, 2026
Merged

fix(data-imports): stop duckgres enablement crashing on non-UUID control-plane org ids#74082
Gilbert09 merged 2 commits into
masterfrom
posthog-code/fix-duckgres-enablement-uuid-crash

Conversation

@Gilbert09

Copy link
Copy Markdown
Member

Problem

Error tracking surfaced a ValidationError: "%(value)s" is not a valid UUID. raised from duckgres_sink_enablement() in products/warehouse_sources/backend/temporal/data_imports/pipelines/pipeline_v3/duckgres/enablement.py, called from the batch sink consumer's _enabled_team_ids().

duckgres_sink_enablement() reads every team row from the managed-warehouse control plane, then queries DuckgresServer to fetch each org's sink concurrency budget:

budgets = {
    str(org_id): sink_max_concurrency
    for org_id, sink_max_concurrency in DuckgresServer.objects.filter(
        organization_id__in={row.organization_id for row in rows}
    ).values_list("organization_id", "sink_max_concurrency")
}

row.organization_id is a raw string reported by the control plane, and the control plane isn't guaranteed to report a UUID there — it has rows for internal test/dev servers keyed by human-readable slugs instead. organization_id is a UUID foreign key on DuckgresServer, so passing a non-UUID value into __in makes Django try to validate every value while building the query, and it raises before ever reaching the org-id match-up a few lines down that would otherwise have filtered the row out. One bad row anywhere in the control plane's response aborted the enablement refresh fleet-wide.

The sampled exception events all lacked any warehouse_sources_* sync context, consistent with the failure coming from this background enablement refresh rather than any specific source/schema/sync.

Changes

  • enablement.py: build the DuckgresServer filter from the app DB's own resolved organization ids (team_info, already validated UUIDs from the Team table) instead of the control plane's raw organization_id strings. Rows for orgs the app DB doesn't recognize already get skipped by the existing org-id match-up right after; they just shouldn't be able to crash the query that runs before it.

How did you test this code?

  • uv run ruff check / ruff format --check — clean.
  • uv run mypy --cache-fine-grained . (scoped to the two changed files) — clean.
  • Added test_duckgres_sink_enablement_ignores_non_uuid_control_plane_org_ids to test_enablement.py: two control-plane rows for the same team, one with a valid org id and one with a non-UUID slug (mirroring the real control-plane data that triggered this). Asserts the refresh doesn't raise and still resolves the valid team.
  • This sandbox has no Postgres/ClickHouse by default, so I set up a local Postgres + a modern ClickHouse build to actually exercise the DB-backed path rather than reasoning about it in the abstract. Against the pre-fix code, a standalone script reproducing the same control-plane shape raises the exact production error (django.core.exceptions.ValidationError: ['"value" is not a valid UUID.']) at the same call site; against the fix, the same script resolves the enablement result without error. pytest itself for this file needs the full ClickHouse cluster config (shard/replica macros) that the dev docker-compose stack provides, which the standalone repro didn't need since it doesn't touch ClickHouse.

Docs update

N/A — internal reliability fix to a background cache refresh, no user-facing or documented behavior change.

🤖 Agent context

Autonomy: Fully autonomous

I (Claude, via PostHog Code) triaged this from a PostHog error-tracking issue with no human steering the investigation. Pulled the issue's stack trace, impact, and sampled $exception events (including captured code variables showing the actual control-plane row values) via the PostHog MCP error-tracking tools, confirmed the sampled events all lacked warehouse_sources_* sync context, then read enablement.py to trace the raise site. The captured rows variable on the exception events showed the exact non-UUID organization_id values from the control plane, which made the root cause unambiguous.

Checked for duplicates: searched open PRs by exception message, module path, and function/file name, and reviewed the maintainer's (Gilbert09) own open PRs. Found two related-but-distinct PRs in this same file (#73975, #73995) that both fix a different bug — the RuntimeError raised when the control plane is unreachable (rows is None) causing a retry storm — neither touches this UUID-filter crash, so this isn't a duplicate.

Invoked /writing-tests before adding the regression test.

…rol-plane org ids

The duckgres batch sink's per-poll enablement refresh filtered DuckgresServer
rows by the managed-warehouse control plane's raw organization_id strings,
which aren't guaranteed to be valid UUIDs (the control plane has rows keyed by
human-readable slugs, e.g. for internal test/dev servers). Passing one of
those straight into a Django ORM filter against a UUID foreign key raised a
ValidationError and aborted the whole refresh for every team, not just the
mismatched row.

Filter by the app DB's own resolved organization ids instead, which are
already guaranteed to be valid UUIDs.

Branch: posthog-code/fix-duckgres-enablement-uuid-crash

Generated-By: PostHog Code
Task-Id: 94dc4a2d-02ac-452b-89cf-41e90a414c50
@github-actions

Copy link
Copy Markdown
Contributor

Hey @Gilbert09! 👋

It looks like your git author email on this PR isn't your @posthog.com address (owerstom@gmail.com). Since you're on the PostHog team, it's worth pointing your local git author email at your @posthog.com address. Why it matters:

  • Consistent work identity in git history — internal tooling that attributes commits to team members keys off your @posthog.com address.
  • Keeps team contributions easy to tell apart from external community ones when scanning history.

You can fix it for this repo with:

git config user.email "you@posthog.com"

Or set it globally with git config --global user.email "you@posthog.com". No need to redo this PR — just a nudge for next time. 🙂

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team July 27, 2026 23:21
@Gilbert09 Gilbert09 added the stamphog Request AI approval (no full review) label Jul 27, 2026 — with PostHog

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Small, well-scoped reliability fix: swaps an unvalidated control-plane string into the UUID FK filter for the app DB's own already-validated org ids, preventing a crash; includes a regression test and matches its description. Not risky territory, author is on the owning team.

  • 👍 on the PR from hex-security-app[bot].
Gate mechanics and policy version
Gate Result
prerequisites all clear
deny-list no deny categories matched
size 6L, 1F substantive, 32L/2F incl. docs/generated/snapshots — within ceiling
tier T1-agent / T1b-small (32L, 2F, single-area, fix)
stamphog 2.0.0b3 .stamphog/policy.yml @ f11de4a · reviewed head 6ff807d

…crash

Generated-By: PostHog Code
Task-Id: 25b1a6d9-ec9d-44f1-8eeb-5ed3825dda0b
@Gilbert09
Gilbert09 merged commit 9c8abf0 into master Jul 28, 2026
235 checks passed
@Gilbert09
Gilbert09 deleted the posthog-code/fix-duckgres-enablement-uuid-crash branch July 28, 2026 09:41
@deployment-status-posthog

deployment-status-posthog Bot commented Jul 28, 2026

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-07-28 10:10 UTC Run
prod-us ✅ Deployed 2026-07-28 10:24 UTC Run
prod-eu ✅ Deployed 2026-07-28 10:28 UTC Run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stamphog Request AI approval (no full review)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant