Skip to content

Bug-2017254: Handle null signatures for PerfCompareResults#9254

Open
moijes12 wants to merge 2 commits intomozilla:masterfrom
moijes12:bug2017254
Open

Bug-2017254: Handle null signatures for PerfCompareResults#9254
moijes12 wants to merge 2 commits intomozilla:masterfrom
moijes12:bug2017254

Conversation

@moijes12
Copy link
Contributor

@moijes12 moijes12 commented Feb 25, 2026

Background

Analysis

  • Calls from PerfCompare to Treeherder's perfcompare/results api endpoint were returning Server Error 500
  • On debugging further, it was observed that this was reproducible if either the base_parent_signature or new_parent_signature parameter was set to null
  • The calls to _get_signatures were throwing exceptions as new_parent_signature was set to null in the caller.
  • The PerfCompareResultsQueryParamsSerializer allows null to be passed for signatures. The signatures are defined as CharFields. @Archaeopteryx @camd : Why is the field created as Charfield while the signatures in other serializers for the Performance data are defined as Integerfield ? What was the reasoning behind it ?
  • Filtering on null results in an error as it expects as integer. The null is also not mapped to None.

Proposed Fix

Added validators to map null to None if null is passed for base or parent signatures in api call to PerfCompareResults.

Fixes Bug-2017254

@Archaeopteryx @kala-moz @gmierz @beatrice-acasandrei

Copy link
Contributor Author

@moijes12 moijes12 left a comment

Choose a reason for hiding this comment

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

@gmierz Thanks for pointing this out. This is fixed in latest commit.

Copy link
Contributor Author

@moijes12 moijes12 left a comment

Choose a reason for hiding this comment

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

If value is None, why are we converting it to None? Seems unnecessary.

@gmierz Thanks for pointing this out. This is fixed in latest commit.

moijes12 added 2 commits March 3, 2026 12:49
Added validators to map `null` to `None` if `null` is passed
for base or parent signatures in api call to PerfCompareResults.
Remove unnecessary None check for parent signature in
Serializer.
Copy link
Contributor Author

@moijes12 moijes12 left a comment

Choose a reason for hiding this comment

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

I have rebased my changes over the latest changes in the master branch.

@moijes12 moijes12 requested a review from gmierz March 4, 2026 15:53
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