feat(preprod): Add preprod artifact upload endpoint #92528

NicoHinderling · 2025-05-29T23:20:49Z

Upload endpoint described in the emerge size integration proposal

Most of the logic is pretty similar to the debug file and sourcemap upload endpoints

in addition to the unit tests, I've manually tested uploading works via my test sentry-cli branch

codecov · 2025-05-29T23:40:04Z

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #92528      +/-   ##
==========================================
+ Coverage   87.90%   87.91%   +0.01%     
==========================================
  Files       10251    10255       +4     
  Lines      587928   588565     +637     
  Branches    22830    22830              
==========================================
+ Hits       516792   517422     +630     
- Misses      70690    70697       +7     
  Partials      446      446

rbro112 · 2025-05-30T16:59:13Z

src/sentry/api/endpoints/organization_preprod_artifact_assemble.py

+                        "type": "array",
+                        "items": {"type": "string", "pattern": "^[0-9a-f]{40}$"},
+                    },
+                    # Optional metadata


Is this where we might need Git sha or other git info?

yes, after writing this doc, I think it's most likely that we'd just include the git info in the request body

JoshFerge

did we end up creating a 'prepod' app folder? is there any reason why the endpoint and task can't live there instead of the catchall folders?

NicoHinderling · 2025-05-30T20:11:24Z

did we end up creating a 'prepod' app folder? is there any reason why the endpoint and task can't live there instead of the catchall folders?

good catch, I can move the endpoint into it. The task file already existed though (just adding a method into it), so I'll plan to leave it as is assuming that's kosher 🙏

JoshFerge · 2025-05-30T20:20:35Z

did we end up creating a 'prepod' app folder? is there any reason why the endpoint and task can't live there instead of the catchall folders?

good catch, I can move the endpoint into it. The task file already existed though (just adding a method into it), so I'll plan to leave it as is assuming that's kosher 🙏

ah yup that makes sense. thanks! also feel free to define your own urls file and import them into the main one like here:

sentry/src/sentry/workflow_engine/endpoints/urls.py

Line 1 in df8e01d

from django.urls import re_path

trevor-e · 2025-05-30T22:14:50Z

src/sentry/preprod/api/endpoints/organization_preprod_artifact_assemble.py

+
+@region_silo_endpoint
+class ProjectPreprodArtifactAssembleEndpoint(ProjectEndpoint):
+    owner = ApiOwner.OWNERS_INGEST


This should probably be for our team.

trevor-e · 2025-05-30T22:19:37Z

src/sentry/preprod/api/endpoints/organization_preprod_artifact_assemble.py

+                    "file_name": data.get("file_name"),
+                    "sha": data.get("sha"),
+                    "build_configuration": data.get("build_configuration"),
+                    "extras": data.get("extras"),


extras shouldn't be part of the external API, these are more for our own internal use. We had a few use-cases in Emerge that were locked down to org42, but let's try to avoid that here if possible for as long as we can.

trevor-e · 2025-05-30T22:21:17Z

src/sentry/tasks/assemble.py

+        # 4. Update state to PROCESSED when done (also update the date_built value to reflect when the artifact was built, among other fields)
+
+    except Exception as e:
+        logger.exception("Failed to assemble preprod artifact")


Can you record the project/org?

trevor-e · 2025-05-30T22:22:14Z

src/sentry/preprod/api/endpoints/organization_preprod_artifact_assemble.py

+                    "project_id": project.id,
+                    "checksum": checksum,
+                    "chunks": chunks,
+                    "file_name": data.get("file_name"),


I think we removed filename in our API a while back, do you think the UUID is fine?

hah, it's still in our docs. yea I talked w hector about this and neither of us felt strongly. We can just auto-gen a file name and remove this param 👍

trevor-e · 2025-05-30T22:23:19Z

src/sentry/preprod/api/endpoints/organization_preprod_artifact_assemble.py

+                    "checksum": checksum,
+                    "chunks": chunks,
+                    "file_name": data.get("file_name"),
+                    "sha": data.get("sha"),


nit: since this assemble api is doing some checksum stuff, maybe this should be git_sha?

loewenheim · 2025-06-02T07:25:21Z

src/sentry/preprod/api/endpoints/organization_preprod_artifact_assemble.py

+            if state == ChunkFileState.OK:
+                return Response({"state": state, "detail": detail, "missingChunks": []})
+            elif state is not None:
+                return Response({"state": state, "detail": detail, "missingChunks": []})


The if and elif branches are the same, so this should be equivalent to

Suggested change

if state == ChunkFileState.OK:

return Response({"state": state, "detail": detail, "missingChunks": []})

elif state is not None:

return Response({"state": state, "detail": detail, "missingChunks": []})

if state is not None:

return Response({"state": state, "detail": detail, "missingChunks": []})

cmanallen

Not commenting on the Emerge specific code. I'll let your team decide what Emerge-specific code should do and if its properly covered. Unfortunately I'm not on any of the teams which are required reviewers. @wedamija was interested in reviewing some of the Emerge code and probably will get you past the review block.

General comments. You don't have to change anything. There are choices you can make. I know the endpoint file is taking inspiration from related files so the tests we have here reflect the choices made by others. But consider the lack of unit test coverage for this pull request. We require a database and Django to test your schema definition, for example. It might simplify your test setup to pull that out into a function. You can test it 100 different ways in a millisecond. I would say as much as possible make this endpoint unit testable. This is a core part of your workflows so having absolute certainty on how things behave would be important for me. Though you're welcome to make a choice that suits your needs.

src/sentry/api/api_owners.py

src/sentry/preprod/api/endpoints/organization_preprod_artifact_assemble.py

cmanallen · 2025-06-01T17:34:32Z

src/sentry/tasks/assemble.py

+        # where next set of changes will happen
+        # TODO: Trigger artifact processing (size analysis, etc.)
+        # This is where you'd add logic to:
+        # 1. create_or_update a new row in the Commit table as well (once base_sha is added as a column to it)
+        # 2. Detect artifact type (iOS/Android/etc.)
+        # 3. Queue processing tasks
+        # 4. Update state to PROCESSED when done (also update the date_built value to reflect when the artifact was built, among other fields)


This should probably be filled in before merging. Is this task useful on its own without the TODOs?

src/sentry/preprod/api/endpoints/organization_preprod_artifact_assemble.py

src/sentry/tasks/assemble.py

cmanallen · 2025-06-01T18:09:06Z

src/sentry/tasks/assemble.py

+        # 3. Queue processing tasks
+        # 4. Update state to PROCESSED when done (also update the date_built value to reflect when the artifact was built, among other fields)
+
+    except Exception as e:


If possible this should be more specific. But logging out to Sentry (as you did below) makes this less of a concern than the previous comment on this.

trevor-e · 2025-06-02T17:47:46Z

src/sentry/preprod/tasks.py

+        # Handle build configuration
+        build_config = None
+        if build_configuration:
+            build_config, _ = PreprodBuildConfiguration.objects.get_or_create(


One other thing, this and PreprodArtifact should likely be created together as part of a transaction.

cmanallen · 2025-06-03T13:55:08Z

tests/sentry/preprod/api/endpoints/test_organization_preprod_artifact_assemble.py

+class ValidatePreprodArtifactSchemaTest(TestCase):
+    """Unit tests for schema validation function - no database required."""
+
+    def test_valid_minimal_schema(self):
+        """Test valid minimal schema passes validation."""
+        data = {"checksum": "a" * 40, "chunks": []}
+        body = orjson.dumps(data)
+        result, error = validate_preprod_artifact_schema(body)
+        assert error is None
+        assert result == data


For unit tests like these you don't need the class structure. You can just define a test:

def test_something(): ...

The test_ prefix (in the tests directory) is a signal to pytest that it should execute that function. There's all sorts of other neat things you can do like bring in fixtures or parameterize your tests. For an example take a look at this test by Tony: https://github.com/getsentry/sentry/pull/92599/files#diff-76fbf98d7d80cafdea351cdb466743d2fa5b20afdb3951d4154406e111dc0805R1088-R1107

tests/sentry/preprod/api/endpoints/test_organization_preprod_artifact_assemble.py

src/sentry/preprod/api/endpoints/organization_preprod_artifact_assemble.py

cmanallen · 2025-06-03T14:54:17Z

@getsentry/issue-workflow Ping.

NicoHinderling · 2025-06-04T00:10:13Z

@getsentry/issue-workflow Ping.

synced w malachi offline 👍

sentry-io · 2025-06-04T16:42:13Z

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

‼️ Exception: Simulated failure unittest.mock in _execute_mock_call View Issue

_{Did you find this useful? React with a 👍 or 👎}

Upload endpoint described in the [emerge size integration proposal](https://www.notion.so/sentry/Emerge-Size-Integration-Technical-Proposal-1ec8b10e4b5d801d9bc4cf8c7cc5ad1b?d=1fa8b10e4b5d80df9ed5001c55e19b89#1fa8b10e4b5d80dba610cb7273434b0d) Most of the logic is pretty similar to the debug file and sourcemap upload endpoints in addition to the unit tests, I've manually tested uploading works via my [test sentry-cli branch](getsentry/sentry-cli#2533)

Adds the upload functionality to the new `mobile-app upload` command. Tested E2E with new API preprod/assemble endpoint added in getsentry/sentry#92528. Also added some basic tests to ensure a minimal APK is successfully uploaded and that it skips uploading if the server already has the chunks.

@JoshFerge

…eprod endpoints (#93825) #92528 (comment) as suggested by @JoshFerge a few weeks back. We're going to add a few more endpoints so it makes sense to have them all in the same self contained file

@JoshFerge

…eprod endpoints (#93825) #92528 (comment) as suggested by @JoshFerge a few weeks back. We're going to add a few more endpoints so it makes sense to have them all in the same self contained file

github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label May 29, 2025

vercel bot deployed to Preview May 29, 2025 23:22 View deployment

rbro112 reviewed May 30, 2025

View reviewed changes

vercel bot deployed to Preview May 30, 2025 17:41 View deployment

vercel bot deployed to Preview May 30, 2025 19:51 View deployment

NicoHinderling changed the title ~~[wip] new preprod artifact upload endpoint~~ feat(preprod): Add preprod artifact upload endpoint May 30, 2025

vercel bot deployed to Preview May 30, 2025 19:57 View deployment

NicoHinderling marked this pull request as ready for review May 30, 2025 20:02

NicoHinderling requested review from a team as code owners May 30, 2025 20:02

JoshFerge reviewed May 30, 2025

View reviewed changes

NicoHinderling requested a review from a team as a code owner May 30, 2025 20:15

vercel bot deployed to Preview May 30, 2025 20:17 View deployment

vercel bot deployed to Preview May 30, 2025 20:34 View deployment

NicoHinderling force-pushed the new-preprod-artifact-endpoint branch from 104af77 to 9a61bbf Compare May 30, 2025 21:51

vercel bot deployed to Preview May 30, 2025 21:53 View deployment

trevor-e reviewed May 30, 2025

View reviewed changes

vercel bot deployed to Preview May 30, 2025 22:43 View deployment

loewenheim reviewed Jun 2, 2025

View reviewed changes

cmanallen reviewed Jun 2, 2025

View reviewed changes

vercel bot deployed to Preview June 2, 2025 17:40 View deployment

vercel bot deployed to Preview June 2, 2025 17:42 View deployment

trevor-e reviewed Jun 2, 2025

View reviewed changes

vercel bot deployed to Preview June 2, 2025 21:13 View deployment

NicoHinderling added 2 commits June 2, 2025 14:16

new preprod artifact upload endpoint

b3a7d58

trevor feedback

0c44c72

NicoHinderling added 4 commits June 2, 2025 14:16

initial comments from colton

648a44c

move assemble test as well

560e97a

add transaction + update tests

4d51928

fix patch typo in tests

13cf722

NicoHinderling force-pushed the new-preprod-artifact-endpoint branch from ecb26de to 13cf722 Compare June 2, 2025 21:57

vercel bot deployed to Preview June 2, 2025 21:59 View deployment

cmanallen reviewed Jun 3, 2025

View reviewed changes

cmanallen approved these changes Jun 3, 2025

View reviewed changes

NicoHinderling added 2 commits June 3, 2025 10:41

fix type issue in test file

1f7c4d7

fix the codeowner issue too

b113c18

NicoHinderling requested a review from a team as a code owner June 3, 2025 17:43

vercel bot deployed to Preview June 3, 2025 17:44 View deployment

NicoHinderling merged commit 055b33d into master Jun 4, 2025
61 checks passed

NicoHinderling deleted the new-preprod-artifact-endpoint branch June 4, 2025 00:10

rbro112 mentioned this pull request Jun 10, 2025

ref(mobile-app): Wireup mobile app upload getsentry/sentry-cli#2542

Merged

NicoHinderling mentioned this pull request Jun 18, 2025

chore(preprod): move endpoint to separate file to share with other preprod endpoints #93825

Merged

github-actions bot locked and limited conversation to collaborators Jun 20, 2025

Uh oh!

feat(preprod): Add preprod artifact upload endpoint #92528

feat(preprod): Add preprod artifact upload endpoint #92528

Uh oh!

Conversation

NicoHinderling commented May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JoshFerge left a comment

Choose a reason for hiding this comment

Uh oh!

NicoHinderling commented May 30, 2025

Uh oh!

JoshFerge commented May 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cmanallen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cmanallen commented Jun 3, 2025

Uh oh!

NicoHinderling commented Jun 4, 2025

Uh oh!

Uh oh!

sentry-io bot commented Jun 4, 2025

Suspect Issues

Uh oh!

Uh oh!

NicoHinderling commented May 29, 2025 •

edited

Loading

codecov bot commented May 29, 2025 •

edited

Loading