-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
bug(preprod): Create PENDING record for comparisons in POST handler to prevent empty state in compare page #101454
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…y state in compare page
This stack of pull requests is managed by Graphite. Learn more about stacking. |
| status: str | ||
| message: str | ||
| existing_comparisons: list[SizeAnalysisComparison] | None | ||
| comparisons: list[SizeAnalysisComparison] | None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't used by the frontend currently, so no need to update any FE types
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #101454 +/- ##
=========================================
Coverage 81.02% 81.03%
=========================================
Files 8701 8703 +2
Lines 385942 386279 +337
Branches 24409 24409
=========================================
+ Hits 312726 313010 +284
- Misses 72865 72918 +53
Partials 351 351 |
| comparison.state = PreprodArtifactSizeComparison.State.PROCESSING | ||
| comparison.save() | ||
| else: | ||
| comparison = PreprodArtifactSizeComparison.objects.create( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be considered an error case now instead of creating a new object? Seems like we should always expect a PENDING comparison to exist so that the frontend page is navigable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, I agree, will update
| "base_artifact_size_metric_id": base_size_metric.id, | ||
| }, | ||
| ) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't tag the line number but I think the error handling we have should now update the comparison to PreprodArtifactSizeComparison.State.FAILED for cases like except File.DoesNotExist. Right now it just returns from the function and the comparison is left PENDING. Eventually our timeout logic will catch it but will take a while.
…aking size comparisons (#101553) I added PENDING records for manual comparisons yesterday in #101454, but I forgot to add for CI triggered comparisons. This adds PENDING comparisons for CI comparisons and fixes a few other minor bugs. Also improves our tests to validate that the DB models are properly created after running comparison tasks/functions, which should catch similar bugs like these in the future.
…o prevent empty state in compare page (#101454)

Creates a PENDING record for all comparisons as part of the POST compare endpoint (manual triggering). This ensures that when a user is redirected to the compare page after triggering a comparison, they no longer see an empty state and instead see the processing state.
Resolves EME-388