chore(warehouse-sources): split run_post_load_operations into steps - #74671
Conversation
|
😎 Merged successfully - details. |
|
Hey @estefaniarabadan! 👋 It looks like your git author email on this PR isn't your
You can fix it for this repo with: git config user.email "you@posthog.com"Or set it globally with |
|
Reviews (1): Last reviewed commit: "chore(warehouse-sources): split run_post..." | Re-trigger Greptile |
|
/trunk merge |
🤖 CI report
|
| File | Patch | Uncovered changed lines |
|---|---|---|
products/warehouse_sources/backend/temporal/data_imports/pipelines/common/load.py |
88.6% | 483–484, 491, 553–554 |
🤖 Agents: add a test covering the lines above, or note why under "How did you test this code?". Machine-readable gap list: the patch-coverage artifact on this run (gh run download 30454963734 -n patch-coverage), or the coverage-data block at the end of this comment.
Per-product line coverage (touched products)
| Product | Coverage | Lines |
|---|---|---|
demo |
███████████░░░░░░░░░ 56.2% |
1,497 / 2,663 |
tasks |
██████████████░░░░░░ 69.7% |
31,931 / 45,803 |
cdp |
████████████████░░░░ 81.0% |
3,146 / 3,883 |
signals |
████████████████░░░░ 81.1% |
23,842 / 29,387 |
data_modeling |
█████████████████░░░ 85.3% |
7,343 / 8,611 |
notebooks |
█████████████████░░░ 86.0% |
7,794 / 9,060 |
actions |
█████████████████░░░ 86.6% |
717 / 828 |
cohorts |
█████████████████░░░ 86.9% |
5,648 / 6,496 |
product_tours |
██████████████████░░ 87.9% |
1,303 / 1,482 |
data_warehouse |
██████████████████░░ 88.3% |
12,036 / 13,635 |
exports |
██████████████████░░ 88.4% |
6,950 / 7,862 |
dashboards |
██████████████████░░ 89.4% |
5,983 / 6,693 |
engineering_analytics |
██████████████████░░ 89.4% |
6,441 / 7,202 |
conversations |
██████████████████░░ 89.6% |
17,234 / 19,241 |
alerts |
██████████████████░░ 90.0% |
4,342 / 4,827 |
mcp_analytics |
██████████████████░░ 90.2% |
2,883 / 3,198 |
streamlit_apps |
██████████████████░░ 90.7% |
2,630 / 2,901 |
error_tracking |
██████████████████░░ 91.0% |
10,925 / 12,004 |
slack_app |
██████████████████░░ 91.1% |
9,554 / 10,492 |
stamphog |
██████████████████░░ 91.1% |
4,056 / 4,450 |
marketing_analytics |
██████████████████░░ 91.2% |
12,058 / 13,222 |
product_analytics |
███████████████████░ 92.5% |
5,849 / 6,321 |
early_access_features |
███████████████████░ 92.6% |
1,287 / 1,390 |
ai_observability |
███████████████████░ 92.8% |
15,328 / 16,517 |
surveys |
███████████████████░ 93.1% |
5,771 / 6,197 |
web_analytics |
███████████████████░ 93.2% |
14,826 / 15,906 |
posthog_ai |
███████████████████░ 93.2% |
1,326 / 1,422 |
approvals |
███████████████████░ 93.3% |
3,437 / 3,682 |
reminders |
███████████████████░ 93.4% |
468 / 501 |
workflows |
███████████████████░ 93.8% |
6,943 / 7,399 |
endpoints |
███████████████████░ 94.2% |
8,655 / 9,192 |
skills |
███████████████████░ 94.6% |
3,158 / 3,337 |
review_hog |
███████████████████░ 94.6% |
6,912 / 7,303 |
experiments |
███████████████████░ 95.4% |
25,752 / 26,982 |
logs |
███████████████████░ 95.5% |
10,435 / 10,928 |
annotations |
███████████████████░ 96.2% |
732 / 761 |
revenue_analytics |
███████████████████░ 96.3% |
1,887 / 1,960 |
feature_flags |
███████████████████░ 96.4% |
17,371 / 18,023 |
replay_vision |
███████████████████░ 96.4% |
15,861 / 16,455 |
user_interviews |
███████████████████░ 96.5% |
2,638 / 2,734 |
customer_analytics |
███████████████████░ 97.1% |
9,797 / 10,085 |
warehouse_sources |
███████████████████░ 97.2% |
346,882 / 356,786 |
data_catalog |
████████████████████ 97.7% |
2,555 / 2,615 |
pulse |
████████████████████ 98.4% |
2,017 / 2,049 |
Report-only. Patch coverage = changed backend lines covered vs origin/master. Sorted lowest first.
Known gaps: lines covered only by Temporal tests show as uncovered; core line numbers may drift if master changed the same file.
|
/trunk merge |
Split the post-load orchestrator in pipelines/common/load.py into named step functions (delta maintenance, queryable-file publishing, sync bookkeeping, table registration/CDC seeding, dependent views, repartition detection) ahead of routing the v2 pipeline through it. Also: move POST_LOAD_DURATION_SECONDS from pipeline_v3 metrics to a common metrics module so the shared orchestrator no longer imports from v3; fetch file URIs inside the orchestrator after maintenance so the queryable folder serves the compacted layout; return the queryable folder; drop the unused cdc_table_mode parameter.
f37ee0d to
a7705a2
Compare
|
Reviews (2): Last reviewed commit: "chore(warehouse-sources): split run_post..." | Re-trigger Greptile |
|
/trunk merge |
Problem
run_post_load_operationsinpipelines/common/load.pyis the shared post-load orchestrator for the v3 pipeline and CDC companion seeding, but it has grown into a single ~220-line function mixing delta maintenance, S3 publishing, sync bookkeeping, table registration, CDC seeding, view syncing, and repartition detection. The v2 pipeline meanwhile carries its own drifted copy of the same sequence.This is the first of two PRs from the pipeline-separation work (following #74352): make the shared orchestrator legible and reusable so the v2 pipeline can be routed through it next, deleting the fork.
Changes
run_post_load_operationsinto named step functions, preserving the existing operation order and all comments:_run_delta_maintenance,_publish_queryable_files,_finalize_sync_bookkeeping,_register_tables_and_seed,_sync_dependent_views,_maybe_flag_repartition. The orchestrator is now a short readable sequence.POST_LOAD_DURATION_SECONDSfrompipeline_v3/load/metrics.pyto a newpipelines/common/metrics.py. The shared module no longer imports from v3 (a layering inversion). Metric name and labels unchanged, so dashboards are unaffected.file_urisparameter. Callers previously passed URIs listed before compaction ran, so the queryable folder was built from the pre-compaction small-file layout (valid until vacuum, but unoptimized). Now it serves the compacted layout.None). Existing callers ignore it; the v2 routing PR needs it for ducklake'sprepared_queryable_folder.cdc_table_modeparameter, which was never read (the body readsschema.cdc_table_mode), and the redundantdelta_table_helper is not Noneclause in the seed decision (the orchestrator's entry guard already ensures it).Note
Behavior is preserved except the file-listing change above: post-load publishing now reflects the post-compaction file layout instead of the pre-compaction one.
How did you test this code?
pipelines/common/test/: 38 passed (includes the existingrun_post_load_operationsmaintenance-branch suite, unchanged except dropping the removedfile_uriskwarg)pipelines/pipeline_v3/: 493 passeduv run mypy --cache-fine-grained .: no issues in 17,075 filesruff check/ruff formatandhogli ci:preflight --fix: cleanNo new tests: this is a step extraction with no new branches; the existing test suite exercises every step through the public orchestrator.
Automatic notifications
Docs update
No user-facing or documented-workflow changes.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Authored with Claude Code as phase 2 (part 1 of 2) of the warehouse pipelines separation plan, continuing #74352. Skills/checks applied: repo-wide mypy, targeted
hogli testruns,hogli ci:preflight. Decisions: kept the step functions module-private (the orchestrator is the only public entry point); kept the existing inline imports inside the step functions rather than hoisting them, since the unit tests patch through those modules and the inline placement guards against import cycles; folded the existing-queryable-folder lookup into the publish step to keep the orchestrator flat. A follow-up PR routes the v2 pipeline's_post_run_operationsthrough this orchestrator and deletes the duplicated copy.