Skip to content

Commit cf92c2f

Browse files
committed
Run snapshot comparisons when uploads received out-of-order
1 parent 7c7bdcb commit cf92c2f

File tree

3 files changed

+168
-2
lines changed

3 files changed

+168
-2
lines changed

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

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,10 @@
5454
PreprodSnapshotMetrics,
5555
)
5656
from sentry.preprod.snapshots.tasks import compare_snapshots
57-
from sentry.preprod.snapshots.utils import find_base_snapshot_artifact
57+
from sentry.preprod.snapshots.utils import (
58+
find_base_snapshot_artifact,
59+
find_head_snapshot_artifacts_awaiting_base,
60+
)
5861
from sentry.preprod.url_utils import get_preprod_artifact_url
5962
from sentry.preprod.vcs.status_checks.snapshots.tasks import (
6063
create_preprod_snapshot_status_check_task,
@@ -637,6 +640,52 @@ def post(self, request: Request, project: Project) -> Response:
637640
},
638641
)
639642

643+
# Trigger comparisons for any head artifacts that were uploaded before this base.
644+
# Handles possible out-of-order uploads where heads arrive before their base build.
645+
if commit_comparison is not None:
646+
try:
647+
waiting_heads = find_head_snapshot_artifacts_awaiting_base(
648+
organization_id=project.organization_id,
649+
base_sha=commit_comparison.head_sha,
650+
base_repo_name=commit_comparison.head_repo_name,
651+
project_id=project.id,
652+
app_id=artifact.app_id,
653+
artifact_type=artifact.artifact_type,
654+
build_configuration=artifact.build_configuration,
655+
)
656+
for head_artifact in waiting_heads:
657+
head_metrics = head_artifact.preprodsnapshotmetrics
658+
logger.info(
659+
"Found head artifact awaiting base, triggering snapshot comparison",
660+
extra={
661+
"head_artifact_id": head_artifact.id,
662+
"base_artifact_id": artifact.id,
663+
"base_sha": commit_comparison.head_sha,
664+
},
665+
)
666+
try:
667+
PreprodSnapshotComparison.objects.get_or_create(
668+
head_snapshot_metrics=head_metrics,
669+
base_snapshot_metrics=snapshot_metrics,
670+
defaults={"state": PreprodSnapshotComparison.State.PENDING},
671+
)
672+
except IntegrityError:
673+
pass
674+
675+
compare_snapshots.apply_async(
676+
kwargs={
677+
"project_id": project.id,
678+
"org_id": project.organization_id,
679+
"head_artifact_id": head_artifact.id,
680+
"base_artifact_id": artifact.id,
681+
},
682+
)
683+
except Exception:
684+
logger.exception(
685+
"Failed to trigger comparisons for head artifacts awaiting base",
686+
extra={"base_artifact_id": artifact.id},
687+
)
688+
640689
return Response(
641690
{
642691
"artifactId": str(artifact.id),

src/sentry/preprod/snapshots/utils.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from __future__ import annotations
22

33
from sentry.preprod.models import PreprodArtifact, PreprodBuildConfiguration
4+
from sentry.preprod.snapshots.models import PreprodSnapshotComparison
45

56

67
def find_base_snapshot_artifact(
@@ -26,3 +27,37 @@ def find_base_snapshot_artifact(
2627
.order_by("-date_added")
2728
.first()
2829
)
30+
31+
32+
def find_head_snapshot_artifacts_awaiting_base(
33+
organization_id: int,
34+
base_sha: str,
35+
base_repo_name: str,
36+
project_id: int,
37+
app_id: str | None,
38+
artifact_type: str | None,
39+
build_configuration: PreprodBuildConfiguration | None,
40+
) -> list[PreprodArtifact]:
41+
"""Find head snapshot artifacts that were uploaded before their base was available.
42+
43+
When a base artifact is uploaded, its commit_comparison.head_sha is the SHA that waiting
44+
head artifacts have as their commit_comparison.base_sha. This finds those heads so
45+
comparisons can be triggered retroactively.
46+
"""
47+
return list(
48+
PreprodArtifact.objects.filter(
49+
commit_comparison__organization_id=organization_id,
50+
commit_comparison__base_sha=base_sha,
51+
commit_comparison__base_repo_name=base_repo_name,
52+
project_id=project_id,
53+
preprodsnapshotmetrics__isnull=False,
54+
app_id=app_id,
55+
artifact_type=artifact_type,
56+
build_configuration=build_configuration,
57+
)
58+
.exclude(
59+
preprodsnapshotmetrics__snapshot_comparisons_head_metrics__state=PreprodSnapshotComparison.State.SUCCESS,
60+
)
61+
.select_related("preprodsnapshotmetrics")
62+
.order_by("-date_added")
63+
)

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

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
from sentry.models.commitcomparison import CommitComparison
77
from sentry.preprod.models import PreprodArtifact
8-
from sentry.preprod.snapshots.models import PreprodSnapshotMetrics
8+
from sentry.preprod.snapshots.models import PreprodSnapshotComparison, PreprodSnapshotMetrics
99
from sentry.testutils.cases import APITestCase
1010

1111

@@ -250,6 +250,88 @@ def test_snapshot_invalid_sha_format(self) -> None:
250250

251251
assert response.status_code == 400
252252

253+
@patch("sentry.preprod.api.endpoints.preprod_artifact_snapshot.get_preprod_session")
254+
@patch("sentry.preprod.api.endpoints.preprod_artifact_snapshot.compare_snapshots")
255+
def test_base_upload_triggers_comparison_for_waiting_head(
256+
self, mock_compare_snapshots, mock_get_session
257+
) -> None:
258+
"""
259+
When a head snapshot is uploaded before its base, uploading the base should
260+
retroactively trigger a comparison for the waiting head.
261+
"""
262+
head_sha = "a" * 40
263+
base_sha = "b" * 40
264+
repo_name = "owner/repo"
265+
app_id = "com.example.app"
266+
267+
# Simulate a head artifact that was uploaded before its base was available.
268+
# It has a commit_comparison with base_sha pointing to the not-yet-uploaded base.
269+
head_commit_comparison = CommitComparison.objects.create(
270+
organization_id=self.org.id,
271+
head_repo_name=repo_name,
272+
head_sha=head_sha,
273+
base_sha=base_sha,
274+
provider="github",
275+
head_ref="feature-branch",
276+
base_repo_name=repo_name,
277+
)
278+
head_artifact = PreprodArtifact.objects.create(
279+
project=self.project,
280+
state=PreprodArtifact.ArtifactState.UPLOADED,
281+
app_id=app_id,
282+
commit_comparison=head_commit_comparison,
283+
)
284+
head_metrics = PreprodSnapshotMetrics.objects.create(
285+
preprod_artifact=head_artifact,
286+
image_count=1,
287+
extras={
288+
"manifest_key": f"{self.org.id}/{self.project.id}/{head_artifact.id}/manifest.json"
289+
},
290+
)
291+
292+
# No comparison exists yet — the base was missing when the head was uploaded.
293+
assert not PreprodSnapshotComparison.objects.filter(
294+
head_snapshot_metrics=head_metrics
295+
).exists()
296+
297+
# Upload the base snapshot. Its head_sha matches the head artifact's base_sha.
298+
url = self._get_create_url()
299+
data = {
300+
"app_id": app_id,
301+
"head_sha": base_sha,
302+
"provider": "github",
303+
"head_repo_name": repo_name,
304+
"head_ref": "main",
305+
"images": {
306+
"img1": {"display_name": "Screen 1", "width": 375, "height": 812},
307+
},
308+
}
309+
310+
with self.feature("organizations:preprod-snapshots"):
311+
response = self.client.post(url, data, format="json")
312+
313+
assert response.status_code == 200
314+
315+
base_artifact = PreprodArtifact.objects.get(id=response.data["artifactId"])
316+
base_metrics = PreprodSnapshotMetrics.objects.get(preprod_artifact=base_artifact)
317+
318+
# A pending comparison record should have been created linking head to base.
319+
comparison = PreprodSnapshotComparison.objects.get(
320+
head_snapshot_metrics=head_metrics,
321+
base_snapshot_metrics=base_metrics,
322+
)
323+
assert comparison.state == PreprodSnapshotComparison.State.PENDING
324+
325+
# The comparison task should have been queued for the waiting head.
326+
mock_compare_snapshots.apply_async.assert_called_once_with(
327+
kwargs={
328+
"project_id": self.project.id,
329+
"org_id": self.org.id,
330+
"head_artifact_id": head_artifact.id,
331+
"base_artifact_id": base_artifact.id,
332+
}
333+
)
334+
253335

254336
class ProjectPreprodSnapshotGetTest(APITestCase):
255337
def setUp(self) -> None:

0 commit comments

Comments
 (0)