Skip to content

Bug 2034671 - Ingest suite-level replicates from performance data.#9459

Merged
gmierz merged 1 commit intomozilla:masterfrom
gmierz:suite-repls
May 7, 2026
Merged

Bug 2034671 - Ingest suite-level replicates from performance data.#9459
gmierz merged 1 commit intomozilla:masterfrom
gmierz:suite-repls

Conversation

@gmierz
Copy link
Copy Markdown
Collaborator

@gmierz gmierz commented Apr 24, 2026

This patch adds suite-level replicates ingestion for performance data. A unit test is also added for it.

@gmierz gmierz requested a review from florinbilt May 6, 2026 18:58
Copy link
Copy Markdown
Collaborator

@florinbilt florinbilt left a comment

Choose a reason for hiding this comment

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

Hi, thanks for the patch! The change looks functionally correct. I have a few suggestions that I think would make it a bit cleaner.

  1. The new block in treeherder/etl/perf.py (lines 230-245) is almost identical to the existing subtest replicates handling (lines 318-333). Would you consider extracting a small helper to keep both call sites in sync going forward?
  2. if replicates and len(replicates) > 0 — the len(...) > 0 part is redundant since an empty list is already falsy. Just if replicates: would be enough.
  3. Would logger.warning (or logger.error) be more appropriate here?
  4. The new test covers the happy path nicely. Could we also add a quick assertion for the case where suite["replicates"] = []?
  5. I'm not entirely sure what the correct way to do this is, but in the subtest flow, replicate ingestion happens before should_mark_as_multi_commit, while in the new suite-level flow it happens after MultiCommitDatum.objects.create(...).

Overall, the patch looks functional and these are mostly stylistic suggestions, none of them are blockers. Thank you! r+

@gmierz gmierz merged commit e554d8a into mozilla:master May 7, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants