chore(launchpad): Simplify launchpad upload logic#97435
Merged
NicoHinderling merged 5 commits intomasterfrom Aug 8, 2025
Merged
chore(launchpad): Simplify launchpad upload logic#97435NicoHinderling merged 5 commits intomasterfrom
NicoHinderling merged 5 commits intomasterfrom
Conversation
❌ 14 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
f685c75 to
f96d949
Compare
rbro112
approved these changes
Aug 7, 2025
5b6a938 to
d4129ea
Compare
| "Finished preprod artifact assembly", | ||
| "Finished preprod artifact row creation and kafka dispatch", | ||
| extra={ | ||
| "preprod_artifact_id": artifact_id, |
Contributor
There was a problem hiding this comment.
Bug: Log Message Mismatch and Error Handling Flaw
The assemble_preprod_artifact task contains two bugs:
- The final success log message, "Finished preprod artifact row creation and kafka dispatch", is inaccurate as "row creation" occurs prior to this task and "kafka dispatch" happens before the log itself.
- The error handling for updating the
PreprodArtifacttoFAILEDdoes not check if the update was successful. If theartifact_idis invalid or the record doesn't exist, the update silently fails, leaving the artifact in an incorrect state.
rbro112
approved these changes
Aug 7, 2025
|
|
||
| return Response({"state": ChunkFileState.CREATED, "missingChunks": []}) | ||
| return Response( | ||
| {"state": ChunkFileState.OK, "missingChunks": [], "artifactId": artifact_id} |
Member
There was a problem hiding this comment.
I am pretty sure we should only return ChunkFileState.OK once assembly has succeeded. ChunkFileState.CREATED is likely the correct thing to return here
Comment on lines
-124
to
-127
| # Check current assembly status | ||
| state, detail = get_assemble_status(AssembleTask.PREPROD_ARTIFACT, project.id, checksum) | ||
| if state is not None: | ||
| return Response({"state": state, "detail": detail, "missingChunks": []}) |
Member
There was a problem hiding this comment.
We should probably add this back, so that we can still support polling (e.g. as an optional feature) from Sentry CLI
andrewshie-sentry
pushed a commit
that referenced
this pull request
Aug 12, 2025
Create the preprod artifact row without file id set. Then pass it to the background task that can set the value post file assembly Also: - No longer poll while the file is being assembled - No longer worry about saving assemble status values (not used anymore) - Return artifact id still
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Create the preprod artifact row without file id set. Then pass it to the background task that can set the value post file assembly
Also: