diff --git a/src/sentry/preprod/api/endpoints/preprod_artifact_approve.py b/src/sentry/preprod/api/endpoints/preprod_artifact_approve.py index be319a507a3735..6474ec0429b4a3 100644 --- a/src/sentry/preprod/api/endpoints/preprod_artifact_approve.py +++ b/src/sentry/preprod/api/endpoints/preprod_artifact_approve.py @@ -1,7 +1,9 @@ from __future__ import annotations import logging +from collections.abc import Callable from datetime import datetime, timezone +from typing import Any from rest_framework.request import Request from rest_framework.response import Response @@ -29,7 +31,7 @@ "size": PreprodComparisonApproval.FeatureType.SIZE, } -STATUS_CHECK_TASK_MAP = { +STATUS_CHECK_TASK_MAP: dict[PreprodComparisonApproval.FeatureType, Callable[..., Any]] = { PreprodComparisonApproval.FeatureType.SNAPSHOTS: create_preprod_snapshot_status_check_task, PreprodComparisonApproval.FeatureType.SIZE: create_preprod_status_check_task, } diff --git a/src/sentry/preprod/vcs/status_checks/snapshots/tasks.py b/src/sentry/preprod/vcs/status_checks/snapshots/tasks.py index c0b133ff4269c7..be538749f0c5ce 100644 --- a/src/sentry/preprod/vcs/status_checks/snapshots/tasks.py +++ b/src/sentry/preprod/vcs/status_checks/snapshots/tasks.py @@ -21,6 +21,7 @@ format_generated_snapshot_status_check_messages, format_missing_base_snapshot_status_check_messages, format_snapshot_status_check_messages, + format_waiting_for_base_snapshot_status_check_messages, ) from sentry.preprod.vcs.status_checks.status_check_provider import ( GITHUB_STATUS_CHECK_STATUS_MAPPING, @@ -45,6 +46,7 @@ FAIL_ON_REMOVED_OPTION_KEY = "sentry:preprod_snapshot_status_checks_fail_on_removed" FAIL_ON_CHANGED_OPTION_KEY = "sentry:preprod_snapshot_status_checks_fail_on_changed" FAIL_ON_RENAMED_OPTION_KEY = "sentry:preprod_snapshot_status_checks_fail_on_renamed" +MISSING_BASE_GRACE_PERIOD_SECONDS = 600 @instrumented_task( @@ -55,7 +57,10 @@ retry=Retry(times=3, delay=60), ) def create_preprod_snapshot_status_check_task( - preprod_artifact_id: int, caller: str | None = None, **kwargs: Any + preprod_artifact_id: int, + caller: str | None = None, + is_timeout_check: bool = False, + **kwargs: Any, ) -> None: try: preprod_artifact: PreprodArtifact | None = PreprodArtifact.objects.select_related( @@ -202,6 +207,7 @@ def create_preprod_snapshot_status_check_task( return approve_action_identifier: str | None = None + waiting_for_base = False if is_solo: app_ids = {a.app_id for a in all_artifacts if a.app_id} @@ -225,12 +231,28 @@ def create_preprod_snapshot_status_check_task( project=preprod_artifact.project, ) elif commit_comparison.base_sha: - status = StatusCheckStatus.FAILURE - title, subtitle, summary = format_missing_base_snapshot_status_check_messages( - all_artifacts, - snapshot_metrics_map, - project=preprod_artifact.project, - ) + if not is_timeout_check: + waiting_for_base = True + status = StatusCheckStatus.IN_PROGRESS + title, subtitle, summary = format_waiting_for_base_snapshot_status_check_messages( + all_artifacts, + snapshot_metrics_map, + project=preprod_artifact.project, + ) + logger.info( + "preprod.snapshot_status_checks.create.missing_base_grace_period", + extra={ + "artifact_id": preprod_artifact.id, + "countdown": MISSING_BASE_GRACE_PERIOD_SECONDS, + }, + ) + else: + status = StatusCheckStatus.FAILURE + title, subtitle, summary = format_missing_base_snapshot_status_check_messages( + all_artifacts, + snapshot_metrics_map, + project=preprod_artifact.project, + ) else: status = StatusCheckStatus.SUCCESS title, subtitle, summary = format_generated_snapshot_status_check_messages( @@ -288,6 +310,16 @@ def create_preprod_snapshot_status_check_task( approve_action_identifier=approve_action_identifier, ) + if waiting_for_base: + create_preprod_snapshot_status_check_task.apply_async( + kwargs={ + "preprod_artifact_id": preprod_artifact_id, + "caller": "missing_base_timeout", + "is_timeout_check": True, + }, + countdown=MISSING_BASE_GRACE_PERIOD_SECONDS, + ) + def _compute_snapshot_status( artifacts: list[PreprodArtifact], diff --git a/src/sentry/preprod/vcs/status_checks/snapshots/templates.py b/src/sentry/preprod/vcs/status_checks/snapshots/templates.py index b93b5617cb3d9f..b7d313f89512e2 100644 --- a/src/sentry/preprod/vcs/status_checks/snapshots/templates.py +++ b/src/sentry/preprod/vcs/status_checks/snapshots/templates.py @@ -223,6 +223,26 @@ def format_missing_base_snapshot_status_check_messages( return str(title), str(subtitle), str(summary) +def format_waiting_for_base_snapshot_status_check_messages( + artifacts: list[PreprodArtifact], + snapshot_metrics_map: dict[int, PreprodSnapshotMetrics], + project: Project, +) -> tuple[str, str, str]: + if not artifacts: + raise ValueError("Cannot format messages for empty artifact list") + + title = _SNAPSHOT_TITLE_BASE + subtitle = str(_("Waiting for base snapshots...")) + + summary = _format_solo_snapshot_summary(artifacts, snapshot_metrics_map) + summary += "\n\nWaiting for base snapshots to finish uploading. This check will update automatically within ~10 minutes or fail." + + settings_url = _get_settings_url(project) + summary += "\n\n" + _format_configure_link(project, settings_url) + + return str(title), str(subtitle), str(summary) + + def _format_solo_snapshot_summary( artifacts: list[PreprodArtifact], snapshot_metrics_map: dict[int, PreprodSnapshotMetrics], diff --git a/tests/sentry/preprod/vcs/status_checks/snapshots/test_tasks.py b/tests/sentry/preprod/vcs/status_checks/snapshots/test_tasks.py index 539bb85a872589..fc89330fe061c0 100644 --- a/tests/sentry/preprod/vcs/status_checks/snapshots/test_tasks.py +++ b/tests/sentry/preprod/vcs/status_checks/snapshots/test_tasks.py @@ -11,6 +11,7 @@ from sentry.preprod.snapshots.utils import build_changes_map from sentry.preprod.vcs.status_checks.snapshots.tasks import ( _compute_snapshot_status, + create_preprod_snapshot_status_check_task, post_snapshot_status_check_task, ) from sentry.preprod.vcs.status_checks.snapshots.templates import ( @@ -386,3 +387,131 @@ def test_no_provider_returns_early(self, mock_get_client, mock_get_provider): mock_get_client.return_value = (Mock(), Mock()) mock_get_provider.return_value = None self._call_task() + + +@cell_silo_test +class CreateSnapshotStatusCheckGracePeriodTest(SnapshotTasksTestBase): + def _create_solo_head_artifact(self): + commit_comparison = self.create_commit_comparison( + head_sha="head123" + "0" * 33, + base_sha="base456" + "0" * 33, + head_repo_name="getsentry/sentry", + provider="github", + ) + artifact = self.create_preprod_artifact( + project=self.project, + commit_comparison=commit_comparison, + app_id="com.example.app", + ) + PreprodSnapshotMetrics.objects.create(preprod_artifact=artifact, image_count=10) + + prev_comparison = self.create_commit_comparison( + head_sha="prev123" + "0" * 33, + base_sha="prev456" + "0" * 33, + head_repo_name="getsentry/sentry", + provider="github", + ) + prev_artifact = self.create_preprod_artifact( + project=self.project, + commit_comparison=prev_comparison, + app_id="com.example.app", + ) + PreprodSnapshotMetrics.objects.create(preprod_artifact=prev_artifact, image_count=10) + + return artifact + + @patch(f"{TASK_MODULE}.post_snapshot_status_check_task") + @patch(f"{TASK_MODULE}.get_status_check_provider") + @patch(f"{TASK_MODULE}.get_status_check_client") + @patch(f"{TASK_MODULE}.create_preprod_snapshot_status_check_task.apply_async") + def test_missing_base_first_attempt_posts_in_progress( + self, mock_apply_async, mock_get_client, mock_get_provider, mock_post_task + ): + mock_get_client.return_value = (Mock(), Mock()) + mock_get_provider.return_value = Mock() + + artifact = self._create_solo_head_artifact() + + create_preprod_snapshot_status_check_task( + preprod_artifact_id=artifact.id, + caller="test", + ) + + mock_post_task.delay.assert_called_once() + call_kwargs = mock_post_task.delay.call_args[1] + assert call_kwargs["status"] == StatusCheckStatus.IN_PROGRESS.value + + mock_apply_async.assert_called_once() + async_kwargs = mock_apply_async.call_args[1] + assert async_kwargs["kwargs"]["is_timeout_check"] is True + assert async_kwargs["kwargs"]["preprod_artifact_id"] == artifact.id + assert async_kwargs["countdown"] == 600 + + @patch(f"{TASK_MODULE}.post_snapshot_status_check_task") + @patch(f"{TASK_MODULE}.get_status_check_provider") + @patch(f"{TASK_MODULE}.get_status_check_client") + def test_missing_base_timeout_posts_failure( + self, mock_get_client, mock_get_provider, mock_post_task + ): + mock_get_client.return_value = (Mock(), Mock()) + mock_get_provider.return_value = Mock() + + artifact = self._create_solo_head_artifact() + + create_preprod_snapshot_status_check_task( + preprod_artifact_id=artifact.id, + caller="missing_base_timeout", + is_timeout_check=True, + ) + + mock_post_task.delay.assert_called_once() + call_kwargs = mock_post_task.delay.call_args[1] + assert call_kwargs["status"] == StatusCheckStatus.FAILURE.value + + @patch(f"{TASK_MODULE}.post_snapshot_status_check_task") + @patch(f"{TASK_MODULE}.get_status_check_provider") + @patch(f"{TASK_MODULE}.get_status_check_client") + def test_timeout_with_base_arrived_runs_normal_path( + self, mock_get_client, mock_get_provider, mock_post_task + ): + mock_get_client.return_value = (Mock(), Mock()) + mock_get_provider.return_value = Mock() + + artifact = self._create_solo_head_artifact() + metrics = PreprodSnapshotMetrics.objects.get(preprod_artifact=artifact) + + base_cc = self.create_commit_comparison( + head_sha="base456" + "0" * 33, + head_repo_name="getsentry/sentry", + provider="github", + ) + base_artifact = self.create_preprod_artifact( + project=self.project, + commit_comparison=base_cc, + app_id="com.example.app", + ) + base_metrics = PreprodSnapshotMetrics.objects.create( + preprod_artifact=base_artifact, + image_count=10, + ) + PreprodSnapshotComparison.objects.create( + head_snapshot_metrics=metrics, + base_snapshot_metrics=base_metrics, + state=PreprodSnapshotComparison.State.SUCCESS, + images_changed=0, + images_added=0, + images_removed=0, + images_renamed=0, + images_unchanged=10, + images_skipped=0, + ) + + create_preprod_snapshot_status_check_task( + preprod_artifact_id=artifact.id, + caller="missing_base_timeout", + is_timeout_check=True, + ) + + mock_post_task.delay.assert_called_once() + call_kwargs = mock_post_task.delay.call_args[1] + assert call_kwargs["status"] == StatusCheckStatus.SUCCESS.value diff --git a/tests/sentry/preprod/vcs/status_checks/snapshots/test_templates.py b/tests/sentry/preprod/vcs/status_checks/snapshots/test_templates.py index 9a433b9d5a9f36..28776da4843574 100644 --- a/tests/sentry/preprod/vcs/status_checks/snapshots/test_templates.py +++ b/tests/sentry/preprod/vcs/status_checks/snapshots/test_templates.py @@ -11,6 +11,7 @@ format_generated_snapshot_status_check_messages, format_missing_base_snapshot_status_check_messages, format_snapshot_status_check_messages, + format_waiting_for_base_snapshot_status_check_messages, ) from sentry.testutils.cases import TestCase from sentry.testutils.silo import cell_silo_test @@ -1139,3 +1140,46 @@ def test_mixed_approved_and_unapproved_artifacts(self) -> None: assert "✅ Approved" in summary assert "⏳ Needs approval" in summary + + +@cell_silo_test +class SnapshotWaitingForBaseFormattingTest(SnapshotStatusCheckTestBase): + def test_waiting_for_base_single_artifact(self) -> None: + artifact, metrics = self._create_artifact_with_metrics( + app_id="com.example.app", app_name="My App", image_count=24 + ) + snapshot_metrics_map = {artifact.id: metrics} + + title, subtitle, summary = format_waiting_for_base_snapshot_status_check_messages( + [artifact], snapshot_metrics_map, project=self.project + ) + + assert title == "Snapshot Testing" + assert subtitle == "Waiting for base snapshots..." + assert "My App" in summary + assert "24" in summary + assert "✅ Uploaded" in summary + assert "Waiting for base snapshots to finish uploading" in summary + + def test_waiting_for_base_multiple_artifacts(self) -> None: + artifacts = [] + snapshot_metrics_map: dict[int, PreprodSnapshotMetrics] = {} + + for i in range(2): + artifact, metrics = self._create_artifact_with_metrics( + app_id=f"com.example.app{i}", + app_name=f"App {i}", + build_number=i + 1, + image_count=10, + ) + artifacts.append(artifact) + snapshot_metrics_map[artifact.id] = metrics + + title, subtitle, summary = format_waiting_for_base_snapshot_status_check_messages( + artifacts, snapshot_metrics_map, project=self.project + ) + + assert title == "Snapshot Testing" + assert subtitle == "Waiting for base snapshots..." + assert "App 0" in summary + assert "App 1" in summary