add transaction + update tests

Author: NicoHinderling

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

@@ -10,15 +10,60 @@
 from sentry.api.bases.project import ProjectEndpoint, ProjectReleasePermission
 from sentry.debug_files.upload import find_missing_chunks
 from sentry.models.orgauthtoken import is_org_auth_token_auth, update_org_auth_token_last_used
+from sentry.preprod.tasks import assemble_preprod_artifact
 from sentry.tasks.assemble import (
     AssembleTask,
     ChunkFileState,
-    assemble_preprod_artifact,
     get_assemble_status,
     set_assemble_status,
 )
 
 
+def validate_preprod_artifact_schema(request_body: bytes) -> tuple[dict, str | None]:
+    """
+    Validate the JSON schema for preprod artifact assembly requests.
+
+    Returns:
+        tuple: (parsed_data, error_message) where error_message is None if validation succeeds
+    """
+    schema = {
+        "type": "object",
+        "properties": {
+            "checksum": {"type": "string", "pattern": "^[0-9a-f]{40}$"},
+            "chunks": {
+                "type": "array",
+                "items": {"type": "string", "pattern": "^[0-9a-f]{40}$"},
+            },
+            # Optional metadata
+            "git_sha": {"type": "string", "pattern": "^[0-9a-f]{40}$"},
+            "build_configuration": {"type": "string"},
+        },
+        "required": ["checksum", "chunks"],
+        "additionalProperties": False,
+    }
+
+    error_messages = {
+        "checksum": "The checksum field is required and must be a 40-character hexadecimal string.",
+        "chunks": "The chunks field is required and must be provided as an array of 40-character hexadecimal strings.",
+        "git_sha": "The git_sha field must be a 40-character hexadecimal string.",
+        "build_configuration": "The build_configuration field must be a string.",
+    }
+
+    try:
+        data = orjson.loads(request_body)
+        jsonschema.validate(data, schema)
+        return data, None
+    except jsonschema.ValidationError as e:
+        error_message = e.message
+        # Get the field from the path if available
+        if e.path:
+            if field := e.path[0]:
+                error_message = error_messages.get(str(field), error_message)
+        return {}, error_message
+    except (orjson.JSONDecodeError, TypeError):
+        return {}, "Invalid json body"
+
+
 @region_silo_endpoint
 class ProjectPreprodArtifactAssembleEndpoint(ProjectEndpoint):
     owner = ApiOwner.EMERGE_TOOLS
@@ -32,42 +77,9 @@ def post(self, request: Request, project) -> Response:
         Assembles a preprod artifact (mobile build, etc.) and stores it in the database.
         """
         with sentry_sdk.start_span(op="preprod_artifact.assemble"):
-            schema = {
-                "type": "object",
-                "properties": {
-                    "checksum": {"type": "string", "pattern": "^[0-9a-f]{40}$"},
-                    "chunks": {
-                        "type": "array",
-                        "items": {"type": "string", "pattern": "^[0-9a-f]{40}$"},
-                    },
-                    # Optional metadata
-                    "git_sha": {"type": "string", "pattern": "^[0-9a-f]{40}$"},
-                    "build_configuration": {"type": "string"},
-                },
-                "required": ["checksum", "chunks"],
-                "additionalProperties": False,
-            }
-
-            error_messages = {
-                "checksum": "The checksum field is required and must be a 40-character hexadecimal string.",
-                "chunks": "The chunks field is required and must be provided as an array of 40-character hexadecimal strings.",
-                "git_sha": "The git_sha field must be a string.",
-                "build_configuration": "The build_configuration field must be a string.",
-            }
-
-            try:
-                data = orjson.loads(request.body)
-                jsonschema.validate(data, schema)
-            except jsonschema.ValidationError as e:
-                error_message = e.message
-                # Get the field from the path if available
-                if e.path:
-                    if field := e.path[0]:
-                        error_message = error_messages.get(str(field), error_message)
-
+            data, error_message = validate_preprod_artifact_schema(request.body)
+            if error_message:
                 return Response({"error": error_message}, status=400)
-            except (orjson.JSONDecodeError, TypeError):
-                return Response({"error": "Invalid json body"}, status=400)
 
             checksum = data.get("checksum")
             chunks = data.get("chunks", [])

src/sentry/preprod/tasks.py

@@ -1,8 +1,11 @@
 from __future__ import annotations
 
+import datetime
 import logging
 import uuid
 
+from django.db import router, transaction
+
 from sentry.models.organization import Organization
 from sentry.models.project import Project
 from sentry.silo.base import SiloMode
@@ -16,7 +19,7 @@
 
 
 @instrumented_task(
-    name="sentry.tasks.assemble.assemble_preprod_artifact",
+    name="sentry.preprod.tasks.assemble_preprod_artifact",
     queue="assemble",
     silo_mode=SiloMode.REGION,
     taskworker_config=TaskworkerConfig(
@@ -38,6 +41,15 @@ def assemble_preprod_artifact(
     """
     from sentry.preprod.models import PreprodArtifact, PreprodBuildConfiguration
 
+    logger.info(
+        "Starting preprod artifact assembly",
+        extra={
+            "timestamp": datetime.datetime.now().isoformat(),
+            "project_id": project_id,
+            "organization_id": org_id,
+        },
+    )
+
     try:
         organization = Organization.objects.get_from_cache(pk=org_id)
         project = Project.objects.get(id=project_id, organization=organization)
@@ -59,22 +71,22 @@ def assemble_preprod_artifact(
         if assemble_result is None:
             return
 
-        # Handle build configuration
-        build_config = None
-        if build_configuration:
-            build_config, _ = PreprodBuildConfiguration.objects.get_or_create(
+        with transaction.atomic(router.db_for_write(PreprodArtifact)):
+            build_config = None
+            if build_configuration:
+                build_config, _ = PreprodBuildConfiguration.objects.get_or_create(
+                    project=project,
+                    name=build_configuration,
+                )
+
+            # Create PreprodArtifact record
+            preprod_artifact = PreprodArtifact.objects.create(
                 project=project,
-                name=build_configuration,
+                file_id=assemble_result.bundle.id,
+                build_configuration=build_config,
+                state=PreprodArtifact.ArtifactState.UPLOADED,
             )
 
-        # Create PreprodArtifact record
-        preprod_artifact = PreprodArtifact.objects.create(
-            project=project,
-            file_id=assemble_result.bundle.id,
-            build_configuration=build_config,
-            state=PreprodArtifact.ArtifactState.UPLOADED,
-        )
-
         logger.info(
             "Created preprod artifact",
             extra={
@@ -84,6 +96,15 @@ def assemble_preprod_artifact(
             },
         )
 
+        logger.info(
+            "Finished preprod artifact assembly",
+            extra={
+                "timestamp": datetime.datetime.now().isoformat(),
+                "project_id": project_id,
+                "organization_id": org_id,
+            },
+        )
+
         # where next set of changes will happen
         # TODO: Trigger artifact processing (size analysis, etc.)
         # This is where you'd add logic to: