ref(preprod): Inline wrapper functions and extract build_changes_map

Remove single-line wrapper functions _find_existing_comment_id and
_save_pr_comment_result, inlining the calls with the hardcoded
"build_distribution" argument directly at the call sites.

Rename _build_changes_map to build_changes_map and move it (along with
its helper _comparison_has_changes) from the status_checks snapshots
tasks module to sentry.preprod.snapshots.utils, since it is now shared
across multiple production modules.

Author: runningcode

src/sentry/preprod/snapshots/utils.py

@@ -1,7 +1,10 @@
 from __future__ import annotations
 
 from sentry.preprod.models import PreprodArtifact, PreprodBuildConfiguration
-from sentry.preprod.snapshots.models import PreprodSnapshotComparison
+from sentry.preprod.snapshots.models import (
+    PreprodSnapshotComparison,
+    PreprodSnapshotMetrics,
+)
 
 
 def find_base_snapshot_artifact(
@@ -59,3 +62,37 @@ def find_head_snapshot_artifacts_awaiting_base(
         .select_related("preprodsnapshotmetrics")
         .order_by("-date_added")
     )
+
+
+def _comparison_has_changes(
+    comparison: PreprodSnapshotComparison,
+    fail_on_added: bool = False,
+    fail_on_removed: bool = True,
+) -> bool:
+    return (
+        comparison.images_changed > 0
+        or comparison.images_renamed > 0
+        or (fail_on_added and comparison.images_added > 0)
+        or (fail_on_removed and comparison.images_removed > 0)
+    )
+
+
+def build_changes_map(
+    artifacts: list[PreprodArtifact],
+    snapshot_metrics_map: dict[int, PreprodSnapshotMetrics],
+    comparisons_map: dict[int, PreprodSnapshotComparison],
+    fail_on_added: bool = False,
+    fail_on_removed: bool = True,
+) -> dict[int, bool]:
+    changes_map: dict[int, bool] = {}
+    for artifact in artifacts:
+        metrics = snapshot_metrics_map.get(artifact.id)
+        if not metrics:
+            continue
+        comparison = comparisons_map.get(metrics.id)
+        if not comparison or comparison.state != PreprodSnapshotComparison.State.SUCCESS:
+            continue
+        changes_map[artifact.id] = _comparison_has_changes(
+            comparison, fail_on_added, fail_on_removed
+        )
+    return changes_map

src/sentry/preprod/vcs/pr_comments/snapshot_tasks.py

@@ -11,12 +11,12 @@
 from sentry.preprod.integration_utils import get_commit_context_client
 from sentry.preprod.models import PreprodArtifact, PreprodComparisonApproval
 from sentry.preprod.snapshots.models import PreprodSnapshotComparison, PreprodSnapshotMetrics
+from sentry.preprod.snapshots.utils import build_changes_map
 from sentry.preprod.vcs.pr_comments.snapshot_templates import format_snapshot_pr_comment
 from sentry.preprod.vcs.pr_comments.tasks import find_existing_comment_id, save_pr_comment_result
 from sentry.preprod.vcs.status_checks.snapshots.tasks import (
     FAIL_ON_ADDED_OPTION_KEY,
     FAIL_ON_REMOVED_OPTION_KEY,
-    _build_changes_map,
 )
 from sentry.shared_integrations.exceptions import ApiError
 from sentry.silo.base import SiloMode
@@ -157,7 +157,7 @@ def create_preprod_snapshot_pr_comment_task(
 
         fail_on_added = artifact.project.get_option(FAIL_ON_ADDED_OPTION_KEY, default=False)
         fail_on_removed = artifact.project.get_option(FAIL_ON_REMOVED_OPTION_KEY, default=True)
-        changes_map = _build_changes_map(
+        changes_map = build_changes_map(
             all_artifacts,
             snapshot_metrics_map,
             comparisons_map,

src/sentry/preprod/vcs/pr_comments/tasks.py

@@ -129,7 +129,7 @@ def create_preprod_pr_comment_task(
         if not installable_siblings:
             return
 
-        existing_comment_id = _find_existing_comment_id(all_for_pr)
+        existing_comment_id = find_existing_comment_id(all_for_pr, "build_distribution")
         comment_body = format_pr_comment(installable_siblings, project=artifact.project)
 
         try:
@@ -165,30 +165,15 @@ def create_preprod_pr_comment_task(
             if isinstance(e, ApiError):
                 extra["status_code"] = e.code
             logger.exception("preprod.pr_comments.create.failed", extra=extra)
-            _save_pr_comment_result(cc, success=False, error=e)
+            save_pr_comment_result(cc, "build_distribution", success=False, error=e)
             api_error = e
         else:
-            _save_pr_comment_result(cc, success=True, comment_id=comment_id)
+            save_pr_comment_result(cc, "build_distribution", success=True, comment_id=comment_id)
 
     if api_error is not None:
         raise api_error
 
 
-def _find_existing_comment_id(
-    comparisons: Sequence[CommitComparison],
-) -> str | None:
-    return find_existing_comment_id(comparisons, "build_distribution")
-
-
-def _save_pr_comment_result(
-    commit_comparison: CommitComparison,
-    success: bool,
-    comment_id: str | None = None,
-    error: Exception | None = None,
-) -> None:
-    save_pr_comment_result(commit_comparison, "build_distribution", success, comment_id, error)
-
-
 def find_existing_comment_id(
     comparisons: Sequence[CommitComparison],
     comment_type: str,

src/sentry/preprod/vcs/status_checks/snapshots/tasks.py

@@ -12,6 +12,7 @@
     PreprodComparisonApproval,
 )
 from sentry.preprod.snapshots.models import PreprodSnapshotComparison, PreprodSnapshotMetrics
+from sentry.preprod.snapshots.utils import build_changes_map
 from sentry.preprod.url_utils import get_preprod_artifact_url
 from sentry.preprod.vcs.status_checks.size.tasks import (
     GITHUB_STATUS_CHECK_STATUS_MAPPING,
@@ -147,7 +148,7 @@ def create_preprod_snapshot_status_check_task(
     is_solo = not base_artifact_map
 
     if not is_solo:
-        changes_map = _build_changes_map(
+        changes_map = build_changes_map(
             all_artifacts,
             snapshot_metrics_map,
             comparisons_map,
@@ -315,40 +316,6 @@ def create_preprod_snapshot_status_check_task(
     )
 
 
-def _comparison_has_changes(
-    comparison: PreprodSnapshotComparison,
-    fail_on_added: bool = False,
-    fail_on_removed: bool = True,
-) -> bool:
-    return (
-        comparison.images_changed > 0
-        or comparison.images_renamed > 0
-        or (fail_on_added and comparison.images_added > 0)
-        or (fail_on_removed and comparison.images_removed > 0)
-    )
-
-
-def _build_changes_map(
-    artifacts: list[PreprodArtifact],
-    snapshot_metrics_map: dict[int, PreprodSnapshotMetrics],
-    comparisons_map: dict[int, PreprodSnapshotComparison],
-    fail_on_added: bool = False,
-    fail_on_removed: bool = True,
-) -> dict[int, bool]:
-    changes_map: dict[int, bool] = {}
-    for artifact in artifacts:
-        metrics = snapshot_metrics_map.get(artifact.id)
-        if not metrics:
-            continue
-        comparison = comparisons_map.get(metrics.id)
-        if not comparison or comparison.state != PreprodSnapshotComparison.State.SUCCESS:
-            continue
-        changes_map[artifact.id] = _comparison_has_changes(
-            comparison, fail_on_added, fail_on_removed
-        )
-    return changes_map
-
-
 def _compute_snapshot_status(
     artifacts: list[PreprodArtifact],
     snapshot_metrics_map: dict[int, PreprodSnapshotMetrics],

tests/sentry/preprod/vcs/status_checks/snapshots/test_tasks.py

@@ -2,8 +2,8 @@
 
 from sentry.integrations.source_code_management.status_check import StatusCheckStatus
 from sentry.preprod.snapshots.models import PreprodSnapshotComparison, PreprodSnapshotMetrics
+from sentry.preprod.snapshots.utils import build_changes_map
 from sentry.preprod.vcs.status_checks.snapshots.tasks import (
-    _build_changes_map,
     _compute_snapshot_status,
 )
 from sentry.testutils.cases import TestCase
@@ -58,7 +58,7 @@ def _status_with_changes_map(self, artifact, metrics, approvals=None, **flags):
         artifacts = [artifact]
         metrics_map = {artifact.id: metrics}
         comparisons_map = {metrics.id: _get_comparison(metrics)}
-        changes_map = _build_changes_map(artifacts, metrics_map, comparisons_map, **flags)
+        changes_map = build_changes_map(artifacts, metrics_map, comparisons_map, **flags)
         return _compute_snapshot_status(
             artifacts, metrics_map, comparisons_map, approvals or {}, changes_map
         )
@@ -102,14 +102,14 @@ def test_no_changes_succeeds(self):
 class BuildChangesMapTest(SnapshotTasksTestBase):
     def test_added_ignored_when_flag_off(self):
         artifact, metrics, _ = self._make_artifact_with_comparison(images_added=5)
-        changes_map = _build_changes_map(
+        changes_map = build_changes_map(
             [artifact], {artifact.id: metrics}, {metrics.id: _get_comparison(metrics)}
         )
         assert not any(changes_map.values())
 
     def test_added_detected_when_flag_on(self):
         artifact, metrics, _ = self._make_artifact_with_comparison(images_added=5)
-        changes_map = _build_changes_map(
+        changes_map = build_changes_map(
             [artifact],
             {artifact.id: metrics},
             {metrics.id: _get_comparison(metrics)},
@@ -119,7 +119,7 @@ def test_added_detected_when_flag_on(self):
 
     def test_removed_ignored_when_flag_off(self):
         artifact, metrics, _ = self._make_artifact_with_comparison(images_removed=3)
-        changes_map = _build_changes_map(
+        changes_map = build_changes_map(
             [artifact],
             {artifact.id: metrics},
             {metrics.id: _get_comparison(metrics)},
@@ -129,14 +129,14 @@ def test_removed_ignored_when_flag_off(self):
 
     def test_removed_detected_by_default(self):
         artifact, metrics, _ = self._make_artifact_with_comparison(images_removed=3)
-        changes_map = _build_changes_map(
+        changes_map = build_changes_map(
             [artifact], {artifact.id: metrics}, {metrics.id: _get_comparison(metrics)}
         )
         assert changes_map[artifact.id] is True
 
     def test_changed_always_detected(self):
         artifact, metrics, _ = self._make_artifact_with_comparison(images_changed=2)
-        changes_map = _build_changes_map(
+        changes_map = build_changes_map(
             [artifact], {artifact.id: metrics}, {metrics.id: _get_comparison(metrics)}
         )
         assert changes_map[artifact.id] is True