Skip to content

Commit b5166a2

Browse files
mtopo27claudegetsantry[bot]
authored
ref(preprod): Restructure commit_comparison conditional to if/else (#111500)
## Summary - Flip the early-return guard `if not artifact.commit_comparison: return` into an `if artifact.commit_comparison: ... else: log` block - Pure structural change — no behavior difference - Prepares for a follow-up PR that adds logic to the else branch and after the if/else block ## Test plan - No-op refactor; existing tests pass unchanged --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
1 parent 1d1708e commit b5166a2

File tree

1 file changed

+176
-166
lines changed
  • src/sentry/preprod/size_analysis

1 file changed

+176
-166
lines changed

src/sentry/preprod/size_analysis/tasks.py

Lines changed: 176 additions & 166 deletions
Original file line numberDiff line numberDiff line change
@@ -74,198 +74,208 @@ def compare_preprod_artifact_size_analysis(
7474
return
7575

7676
try:
77-
if not artifact.commit_comparison:
78-
logger.info(
79-
"preprod.size_analysis.compare.artifact_no_commit_comparison",
80-
extra={"artifact_id": artifact_id},
81-
)
82-
return
83-
84-
comparisons: list[dict[str, PreprodArtifactSizeMetrics]] = []
85-
preprod_artifact_status_check_updates: set[int] = {artifact.id}
86-
87-
# Create all comparisons with artifact as head
88-
base_artifact = artifact.get_base_artifact_for_commit().first()
89-
if base_artifact:
90-
if artifact.build_configuration != base_artifact.build_configuration:
91-
logger.info(
92-
"preprod.size_analysis.compare.artifact_different_build_configurations",
93-
extra={"head_artifact_id": artifact_id, "base_artifact_id": base_artifact.id},
77+
if artifact.commit_comparison:
78+
comparisons: list[dict[str, PreprodArtifactSizeMetrics]] = []
79+
preprod_artifact_status_check_updates: set[int] = {artifact.id}
80+
81+
# Create all comparisons with artifact as head
82+
base_artifact = artifact.get_base_artifact_for_commit().first()
83+
if base_artifact:
84+
if artifact.build_configuration != base_artifact.build_configuration:
85+
logger.info(
86+
"preprod.size_analysis.compare.artifact_different_build_configurations",
87+
extra={
88+
"head_artifact_id": artifact_id,
89+
"base_artifact_id": base_artifact.id,
90+
},
91+
)
92+
# Update the status check even though we can't compare to avoid leaving it in a loading state
93+
create_preprod_status_check_task.apply_async(
94+
kwargs={
95+
"preprod_artifact_id": artifact_id,
96+
"caller": "compare_build_config_mismatch",
97+
}
98+
)
99+
return
100+
101+
base_size_metrics_qs = PreprodArtifactSizeMetrics.objects.filter(
102+
preprod_artifact_id=base_artifact.id,
103+
preprod_artifact__project__organization_id=org_id,
104+
preprod_artifact__project_id=project_id,
105+
).select_related("preprod_artifact", "preprod_artifact__mobile_app_info")
106+
base_size_metrics = list(base_size_metrics_qs)
107+
108+
head_size_metrics_qs = PreprodArtifactSizeMetrics.objects.filter(
109+
preprod_artifact_id=artifact_id,
110+
preprod_artifact__project__organization_id=org_id,
111+
preprod_artifact__project_id=project_id,
112+
).select_related(
113+
"preprod_artifact",
114+
"preprod_artifact__mobile_app_info",
115+
"preprod_artifact__commit_comparison",
94116
)
95-
# Update the status check even though we can't compare to avoid leaving it in a loading state
96-
create_preprod_status_check_task.apply_async(
97-
kwargs={
98-
"preprod_artifact_id": artifact_id,
99-
"caller": "compare_build_config_mismatch",
100-
}
117+
head_size_metrics = list(head_size_metrics_qs)
118+
119+
validation_result = can_compare_size_metrics(head_size_metrics, base_size_metrics)
120+
if validation_result.can_compare:
121+
base_metrics_map = build_size_metrics_map(base_size_metrics)
122+
head_metrics_map = build_size_metrics_map(head_size_metrics)
123+
124+
for key, base_metric in base_metrics_map.items():
125+
matching_head_size_metric = head_metrics_map.get(key)
126+
if matching_head_size_metric:
127+
logger.info(
128+
"preprod.size_analysis.compare.create_comparison",
129+
extra={
130+
"head_artifact_id": artifact_id,
131+
"base_artifact_id": base_artifact.id,
132+
},
133+
)
134+
comparisons.append(
135+
{
136+
"head_metric": matching_head_size_metric,
137+
"base_metric": base_metric,
138+
},
139+
)
140+
else:
141+
logger.info(
142+
"preprod.size_analysis.compare.no_matching_base_size_metric",
143+
extra={
144+
"head_artifact_id": artifact_id,
145+
"size_metric_id": base_metric.id,
146+
},
147+
)
148+
else:
149+
logger.info(
150+
"preprod.size_analysis.compare.cannot_compare_size_metrics",
151+
extra={
152+
"head_artifact_id": artifact_id,
153+
"base_artifact_id": base_artifact.id,
154+
"error_message": validation_result.error_message,
155+
},
156+
)
157+
158+
# Also create comparisons with artifact as base
159+
head_artifacts = artifact.get_head_artifacts_for_commit()
160+
for head_artifact in head_artifacts:
161+
if head_artifact.build_configuration != artifact.build_configuration:
162+
logger.info(
163+
"preprod.size_analysis.compare.head_artifact_different_build_configurations",
164+
extra={
165+
"head_artifact_id": head_artifact.id,
166+
"base_artifact_id": artifact_id,
167+
},
168+
)
169+
continue
170+
171+
head_size_metrics_qs = PreprodArtifactSizeMetrics.objects.filter(
172+
preprod_artifact_id=head_artifact.id,
173+
preprod_artifact__project__organization_id=org_id,
174+
preprod_artifact__project_id=project_id,
175+
).select_related(
176+
"preprod_artifact",
177+
"preprod_artifact__mobile_app_info",
178+
"preprod_artifact__commit_comparison",
101179
)
102-
return
103-
104-
base_size_metrics_qs = PreprodArtifactSizeMetrics.objects.filter(
105-
preprod_artifact_id=base_artifact.id,
106-
preprod_artifact__project__organization_id=org_id,
107-
preprod_artifact__project_id=project_id,
108-
).select_related("preprod_artifact", "preprod_artifact__mobile_app_info")
109-
base_size_metrics = list(base_size_metrics_qs)
110-
111-
head_size_metrics_qs = PreprodArtifactSizeMetrics.objects.filter(
112-
preprod_artifact_id=artifact_id,
113-
preprod_artifact__project__organization_id=org_id,
114-
preprod_artifact__project_id=project_id,
115-
).select_related(
116-
"preprod_artifact",
117-
"preprod_artifact__mobile_app_info",
118-
"preprod_artifact__commit_comparison",
119-
)
120-
head_size_metrics = list(head_size_metrics_qs)
180+
head_size_metrics = list(head_size_metrics_qs)
181+
182+
base_size_metrics_qs = PreprodArtifactSizeMetrics.objects.filter(
183+
preprod_artifact_id=artifact_id,
184+
preprod_artifact__project__organization_id=org_id,
185+
preprod_artifact__project_id=project_id,
186+
).select_related("preprod_artifact", "preprod_artifact__mobile_app_info")
187+
base_size_metrics = list(base_size_metrics_qs)
188+
189+
validation_result = can_compare_size_metrics(head_size_metrics, base_size_metrics)
190+
if not validation_result.can_compare:
191+
logger.info(
192+
"preprod.size_analysis.compare.cannot_compare_size_metrics",
193+
extra={
194+
"head_artifact_id": head_artifact.id,
195+
"base_artifact_id": artifact_id,
196+
"error_message": validation_result.error_message,
197+
},
198+
)
199+
continue
121200

122-
validation_result = can_compare_size_metrics(head_size_metrics, base_size_metrics)
123-
if validation_result.can_compare:
124-
base_metrics_map = build_size_metrics_map(base_size_metrics)
125201
head_metrics_map = build_size_metrics_map(head_size_metrics)
202+
base_metrics_map = build_size_metrics_map(base_size_metrics)
126203

127-
for key, base_metric in base_metrics_map.items():
128-
matching_head_size_metric = head_metrics_map.get(key)
129-
if matching_head_size_metric:
204+
for key, head_metric in head_metrics_map.items():
205+
matching_base_size_metric = base_metrics_map.get(key)
206+
if matching_base_size_metric:
130207
logger.info(
131208
"preprod.size_analysis.compare.create_comparison",
132209
extra={
133-
"head_artifact_id": artifact_id,
134-
"base_artifact_id": base_artifact.id,
210+
"head_artifact_id": head_artifact.id,
211+
"base_artifact_id": artifact.id,
135212
},
136213
)
137214
comparisons.append(
138-
{"head_metric": matching_head_size_metric, "base_metric": base_metric},
215+
{"head_metric": head_metric, "base_metric": matching_base_size_metric},
139216
)
217+
preprod_artifact_status_check_updates.add(head_artifact.id)
140218
else:
141219
logger.info(
142220
"preprod.size_analysis.compare.no_matching_base_size_metric",
143221
extra={
144-
"head_artifact_id": artifact_id,
145-
"size_metric_id": base_metric.id,
222+
"head_artifact_id": head_artifact.id,
223+
"size_metric_id": head_metric.id,
146224
},
147225
)
148-
else:
149-
logger.info(
150-
"preprod.size_analysis.compare.cannot_compare_size_metrics",
151-
extra={
152-
"head_artifact_id": artifact_id,
153-
"base_artifact_id": base_artifact.id,
154-
"error_message": validation_result.error_message,
155-
},
156-
)
157-
158-
# Also create comparisons with artifact as base
159-
head_artifacts = artifact.get_head_artifacts_for_commit()
160-
for head_artifact in head_artifacts:
161-
if head_artifact.build_configuration != artifact.build_configuration:
162-
logger.info(
163-
"preprod.size_analysis.compare.head_artifact_different_build_configurations",
164-
extra={"head_artifact_id": head_artifact.id, "base_artifact_id": artifact_id},
165-
)
166-
continue
167-
168-
head_size_metrics_qs = PreprodArtifactSizeMetrics.objects.filter(
169-
preprod_artifact_id=head_artifact.id,
170-
preprod_artifact__project__organization_id=org_id,
171-
preprod_artifact__project_id=project_id,
172-
).select_related(
173-
"preprod_artifact",
174-
"preprod_artifact__mobile_app_info",
175-
"preprod_artifact__commit_comparison",
176-
)
177-
head_size_metrics = list(head_size_metrics_qs)
178226

179-
base_size_metrics_qs = PreprodArtifactSizeMetrics.objects.filter(
180-
preprod_artifact_id=artifact_id,
181-
preprod_artifact__project__organization_id=org_id,
182-
preprod_artifact__project_id=project_id,
183-
).select_related("preprod_artifact", "preprod_artifact__mobile_app_info")
184-
base_size_metrics = list(base_size_metrics_qs)
185-
186-
validation_result = can_compare_size_metrics(head_size_metrics, base_size_metrics)
187-
if not validation_result.can_compare:
188-
logger.info(
189-
"preprod.size_analysis.compare.cannot_compare_size_metrics",
190-
extra={
191-
"head_artifact_id": head_artifact.id,
192-
"base_artifact_id": artifact_id,
193-
"error_message": validation_result.error_message,
194-
},
195-
)
196-
continue
197-
198-
head_metrics_map = build_size_metrics_map(head_size_metrics)
199-
base_metrics_map = build_size_metrics_map(base_size_metrics)
227+
# Create PENDING comparison records in DB and run comparisons
228+
with transaction.atomic(router.db_for_write(PreprodArtifactSizeComparison)):
229+
for comp in comparisons:
230+
head_metric = comp["head_metric"]
231+
base_metric = comp["base_metric"]
232+
comparison, created = PreprodArtifactSizeComparison.objects.get_or_create(
233+
head_size_analysis=head_metric,
234+
base_size_analysis=base_metric,
235+
organization_id=org_id,
236+
defaults={"state": PreprodArtifactSizeComparison.State.PENDING},
237+
)
200238

201-
for key, head_metric in head_metrics_map.items():
202-
matching_base_size_metric = base_metrics_map.get(key)
203-
if matching_base_size_metric:
204239
logger.info(
205-
"preprod.size_analysis.compare.create_comparison",
240+
"preprod.size_analysis.compare.running_comparison",
206241
extra={
207-
"head_artifact_id": head_artifact.id,
208-
"base_artifact_id": artifact.id,
242+
"head_metric_id": head_metric.id,
243+
"base_metric_id": base_metric.id,
244+
"comparison_created": created,
209245
},
210246
)
211-
comparisons.append(
212-
{"head_metric": head_metric, "base_metric": matching_base_size_metric},
213-
)
214-
preprod_artifact_status_check_updates.add(head_artifact.id)
215-
else:
216-
logger.info(
217-
"preprod.size_analysis.compare.no_matching_base_size_metric",
218-
extra={
219-
"head_artifact_id": head_artifact.id,
220-
"size_metric_id": head_metric.id,
221-
},
247+
_run_size_analysis_comparison(org_id, head_metric, base_metric)
248+
249+
for artifact_id in preprod_artifact_status_check_updates:
250+
# Update all artifact's status check with the new comparison
251+
create_preprod_status_check_task.apply_async(
252+
kwargs={
253+
"preprod_artifact_id": artifact_id,
254+
"caller": "compare_completion",
255+
}
222256
)
223257

224-
# Create PENDING comparison records in DB and run comparisons
225-
with transaction.atomic(router.db_for_write(PreprodArtifactSizeComparison)):
226-
for comp in comparisons:
227-
head_metric = comp["head_metric"]
228-
base_metric = comp["base_metric"]
229-
comparison, created = PreprodArtifactSizeComparison.objects.get_or_create(
230-
head_size_analysis=head_metric,
231-
base_size_analysis=base_metric,
232-
organization_id=org_id,
233-
defaults={"state": PreprodArtifactSizeComparison.State.PENDING},
234-
)
235-
236-
logger.info(
237-
"preprod.size_analysis.compare.running_comparison",
238-
extra={
239-
"head_metric_id": head_metric.id,
240-
"base_metric_id": base_metric.id,
241-
"comparison_created": created,
242-
},
243-
)
244-
_run_size_analysis_comparison(org_id, head_metric, base_metric)
245-
246-
for artifact_id in preprod_artifact_status_check_updates:
247-
# Update all artifact's status check with the new comparison
248-
create_preprod_status_check_task.apply_async(
249-
kwargs={
250-
"preprod_artifact_id": artifact_id,
251-
"caller": "compare_completion",
252-
}
253-
)
254-
255-
try:
256-
artifact_type_name = PreprodArtifact.ArtifactType(artifact.artifact_type).name.lower()
257-
except (ValueError, AttributeError, TypeError):
258-
artifact_type_name = "unknown"
259-
260-
e2e_size_analysis_compare_duration = timezone.now() - artifact.date_added
261-
metrics.distribution(
262-
"preprod.size_analysis.compare.results_e2e",
263-
e2e_size_analysis_compare_duration.total_seconds(),
264-
sample_rate=1.0,
265-
tags={
266-
"artifact_type": artifact_type_name,
267-
},
268-
)
258+
try:
259+
artifact_type_name = PreprodArtifact.ArtifactType(
260+
artifact.artifact_type
261+
).name.lower()
262+
except (ValueError, AttributeError, TypeError):
263+
artifact_type_name = "unknown"
264+
265+
e2e_size_analysis_compare_duration = timezone.now() - artifact.date_added
266+
metrics.distribution(
267+
"preprod.size_analysis.compare.results_e2e",
268+
e2e_size_analysis_compare_duration.total_seconds(),
269+
sample_rate=1.0,
270+
tags={
271+
"artifact_type": artifact_type_name,
272+
},
273+
)
274+
else:
275+
logger.info(
276+
"preprod.size_analysis.compare.artifact_no_commit_comparison",
277+
extra={"artifact_id": artifact_id},
278+
)
269279
finally:
270280
send_size_analysis_webhook(artifact=artifact, organization_id=org_id)
271281

0 commit comments

Comments
 (0)