Bug 2059046 - [Perfherder] Display Sherlock's Suggested Culprit in Alerts View - #9818
Bug 2059046 - [Perfherder] Display Sherlock's Suggested Culprit in Alerts View#9818esanuandra wants to merge 4 commits into
Conversation
✅ Deploy Preview for treeherder ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
fix lint error
233165c to
b348258
Compare
| // ...but the styled detected-push revision span does not | ||
| expect( | ||
| container.querySelector('.fst-italic.text-muted.small'), | ||
| ).toBeNull(); |
There was a problem hiding this comment.
If a Bootstrap version update or a style change renames those classes, the test will silently pass even if the element is incorrectly rendered.
| ).toBeNull(); | |
| expect( | |
| container.querySelector('[data-testid$="suggested-culprit"]'), | |
| ).toBeNull(); |
| }); | ||
|
|
||
| const revisionEl = await waitFor(() => | ||
| getByText(`Suggested culprit: ${revision.slice(0, 12)}`), |
There was a problem hiding this comment.
Nit: Consider extracting this hardcoded slicing value into a constant (e.g., const REVISION_DISPLAY_LENGTH = 12;). Since it’s used frequently throughout the project, this will help keep it consistent and easier to maintain. This can also be addressed later since it involves other files as well.
| for entry in reversed(self.get_backfill_logs()): | ||
| if entry.get("detected_push_id") is not None: | ||
| return { | ||
| "detected_push_id": entry.get("detected_push_id"), |
There was a problem hiding this comment.
| "detected_push_id": entry.get("detected_push_id"), | |
| "detected_push_id": entry["detected_push_id"], |
|
|
||
| def get_detected_push_revision(self, obj): | ||
| detected = obj.get_latest_detected_push() | ||
| return detected["detected_push_revision"] if detected else None |
There was a problem hiding this comment.
When DRF serializes a BackfillRecord, it calls both get_detected_push_id and get_detected_push_revision. Each calls get_latest_detected_push(), which in turn calls json.loads() on the backfill logs. That means the JSON is parsed twice for every record in the API response.
The following approach computes the result once sets both keys manually. This is the DRF-idiomatic way to customize serialization output.
to_representation is a special method built into DRF that every serializer already has. It's the method DRF calls internally when it converts a model object into a Python dict (before turning it into JSON).
Instead of adding two new fields — you're overriding that built-in method to inject your two values after the normal serialization runs.
| return detected["detected_push_revision"] if detected else None | |
| def to_representation(self, instance): | |
| data = super().to_representation(instance) # 1. run normal serialization first | |
| # (all existing fields are in `data`) | |
| detected = instance.get_latest_detected_push() # 2. compute once | |
| data["detected_push_id"] = ... # 3. add field 1 to the dict | |
| data["detected_push_revision"] = ... # 4. add field 2 to the dict | |
| return data # 5. return the final dict |
There are multiple ways to compute only once detected_push_id and detected_push_revision. This is one option, but I'm open to any alternative solutions you'd prefer to explore.
| </span> | ||
| {detectedPushRevision && ( | ||
| <span className="ms-1 fst-italic text-muted small"> | ||
| Suggested culprit: {detectedPushRevision.slice(0, 12)} |
There was a problem hiding this comment.
nit: As a potential follow-up, we could add a direct link to the Jobs View when clicking on the revision.
Archaeopteryx
added
the
perfherder
3 participants
Bug 2059046 - [Perfherder] Display Sherlock's Suggested Culprit in Alerts View
It displays suggested culprit revision next to sherlock hourglass in alert's row
