feat: add command to recompute tree_tests_rollup#1874
feat: add command to recompute tree_tests_rollup#1874gustavobtflores wants to merge 2 commits intokernelci:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a one-off Django management command to (re)populate tree_tests_rollup from existing Tests/Builds/Incidents data, and centralizes the rollup processed-item key hashing helper so both streaming aggregation and backfill paths use the same key generator.
Changes:
- Introduces
populate_tree_tests_rollupmanagement command to recompute/upserttree_tests_rollupand upsert correspondingProcessedListingItemsentries. - Moves
get_rollup_key()intoprocess_pending_helpersand reuses it fromprocess_pending_aggregations. - Tightens typing in
process_pending_helpersto useRollupKeyfor rollup dictionaries.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
backend/kernelCI_app/management/commands/process_pending_aggregations.py |
Imports shared get_rollup_key() instead of defining a local version. |
backend/kernelCI_app/management/commands/populate_tree_tests_rollup.py |
New command that aggregates tests into rollup buckets and writes rollup + processed markers. |
backend/kernelCI_app/management/commands/helpers/process_pending_helpers.py |
Adds shared get_rollup_key() and updates rollup typing to use RollupKey. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1b8f800 to
a3099d0
Compare
a3099d0 to
6467279
Compare
| ) -> None: | ||
| """Merge chunk rollup data into the accumulator.""" | ||
| for key, data in chunk_rollup.items(): | ||
| if key in accumulator: |
There was a problem hiding this comment.
nit: can we just set the default to be a dict full of zeros?
| raise CommandError(f"Checkout with id={checkout_id} not found") | ||
|
|
||
| if limit is not None: | ||
| checkouts_qs = checkouts_qs[:limit] |
There was a problem hiding this comment.
checkouts_qs[:None] should give you the whole list, we can avoid this conditional
6467279 to
52725fe
Compare
MarceloRobert
left a comment
There was a problem hiding this comment.
Seems to have worked well in my tests
52725fe to
3a10c51
Compare
| values = [ | ||
| ( | ||
| *key, | ||
| data["pass_tests"], |
There was a problem hiding this comment.
nit: could we use query named args? to avoid having to create these tuples? It could also improve readability.
Description
Adds a new management command to recompute
tree_tests_rollupandProcessedListingItemsfrom source data. This enables backfilling or rebuilding rollup aggregations when needed, with options to filter by checkout, time range, or limit.Changes
populate_tree_tests_rollupmanagement command with filtering options (--checkout-id,--since-days,--limit) and dry-run modeHow to test
process_pending_aggregationscommand if runningpoetry run python manage.py populate_tree_tests_rollup --since-days=7 --dry-runpoetry run python manage.py populate_tree_tests_rollup --checkout-id=<uuid>process_pending_aggregationsafter completionCloses #1882