Skip to content

Comments

chore(launchpad): Simplify launchpad upload logic#97435

Merged
NicoHinderling merged 5 commits intomasterfrom
simplify-artifact-upload-logic
Aug 8, 2025
Merged

chore(launchpad): Simplify launchpad upload logic#97435
NicoHinderling merged 5 commits intomasterfrom
simplify-artifact-upload-logic

Conversation

@NicoHinderling
Copy link
Contributor

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

@NicoHinderling NicoHinderling requested a review from a team as a code owner August 7, 2025 20:56
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Aug 7, 2025
cursor[bot]

This comment was marked as outdated.

@codecov
Copy link

codecov bot commented Aug 7, 2025

❌ 14 Tests Failed:

Tests completed Failed Passed Skipped
27014 14 27000 590
View the top 3 failed test(s) by shortest run time
tests.sentry.preprod.api.endpoints.test_organization_preprod_artifact_assemble.ProjectPreprodArtifactAssembleTest::test_integration_task_sets_status_api_can_read_it
Stack Traces | 2.21s run time
#x1B[1m#x1B[.../api/endpoints/test_organization_preprod_artifact_assemble.py#x1B[0m:589: in test_integration_task_sets_status_api_can_read_it
    assert response.data["state"] == ChunkFileState.OK
#x1B[1m#x1B[31mE   AssertionError: assert 'not_found' == 'ok'#x1B[0m
#x1B[1m#x1B[31mE     #x1B[0m
#x1B[1m#x1B[31mE     - ok#x1B[0m
#x1B[1m#x1B[31mE     + not_found#x1B[0m
tests.sentry.preprod.api.endpoints.test_organization_preprod_artifact_assemble.ProjectPreprodArtifactAssembleTest::test_check_existing_assembly_status
Stack Traces | 2.29s run time
#x1B[1m#x1B[.../api/endpoints/test_organization_preprod_artifact_assemble.py#x1B[0m:556: in test_check_existing_assembly_status
    assert response.data["state"] == ChunkFileState.OK
#x1B[1m#x1B[31mE   AssertionError: assert 'not_found' == 'ok'#x1B[0m
#x1B[1m#x1B[31mE     #x1B[0m
#x1B[1m#x1B[31mE     - ok#x1B[0m
#x1B[1m#x1B[31mE     + not_found#x1B[0m
tests.sentry.preprod.api.endpoints.test_organization_preprod_artifact_assemble.ProjectPreprodArtifactAssembleTest::test_assemble_basic
Stack Traces | 2.34s run time
#x1B[1m#x1B[.../api/endpoints/test_organization_preprod_artifact_assemble.py#x1B[0m:333: in test_assemble_basic
    assert response.data["state"] == ChunkFileState.CREATED
#x1B[1m#x1B[31mE   AssertionError: assert 'error' == 'created'#x1B[0m
#x1B[1m#x1B[31mE     #x1B[0m
#x1B[1m#x1B[31mE     - created#x1B[0m
#x1B[1m#x1B[31mE     + error#x1B[0m

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@NicoHinderling NicoHinderling force-pushed the simplify-artifact-upload-logic branch from 5b6a938 to d4129ea Compare August 7, 2025 22:01
"Finished preprod artifact assembly",
"Finished preprod artifact row creation and kafka dispatch",
extra={
"preprod_artifact_id": artifact_id,
Copy link
Contributor

Choose a reason for hiding this comment

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

Bug: Log Message Mismatch and Error Handling Flaw

The assemble_preprod_artifact task contains two bugs:

  1. 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.
  2. The error handling for updating the PreprodArtifact to FAILED does not check if the update was successful. If the artifact_id is invalid or the record doesn't exist, the update silently fails, leaving the artifact in an incorrect state.
Fix in Cursor Fix in Web

@NicoHinderling NicoHinderling merged commit b87fc1a into master Aug 8, 2025
63 checks passed
@NicoHinderling NicoHinderling deleted the simplify-artifact-upload-logic branch August 8, 2025 00:59

return Response({"state": ChunkFileState.CREATED, "missingChunks": []})
return Response(
{"state": ChunkFileState.OK, "missingChunks": [], "artifactId": artifact_id}
Copy link
Member

Choose a reason for hiding this comment

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

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": []})
Copy link
Member

Choose a reason for hiding this comment

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

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
@github-actions github-actions bot locked and limited conversation to collaborators Aug 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants