Skip to content

Commit 671b040

Browse files
committed
typing fix
1 parent b6295da commit 671b040

File tree

3 files changed

+41
-68
lines changed

3 files changed

+41
-68
lines changed

src/sentry/incidents/logic.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -929,10 +929,8 @@ def update_alert_rule(
929929
updated_fields["seasonality"] = None
930930
elif detection_type == AlertRuleDetectionType.DYNAMIC:
931931
# NOTE: we set seasonality for EA
932-
updated_query_fields["resolution"] = (
933-
timedelta(minutes=time_window)
934-
if time_window is not None
935-
else timedelta(seconds=snuba_query.time_window)
932+
updated_query_fields["resolution"] = timedelta(
933+
minutes=time_window if time_window is not None else snuba_query.time_window
936934
)
937935
updated_fields["seasonality"] = AlertRuleSeasonality.AUTO
938936
updated_fields["comparison_delta"] = None

src/sentry/incidents/metric_issue_detector.py

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -250,18 +250,21 @@ def _validate_extrapolation_mode(self, extrapolation_mode: ExtrapolationMode) ->
250250
)
251251

252252
def _get_resolution_for_window(
253-
self, time_window_seconds: int, detection_type: str | None, comparison_delta: int | None
253+
self,
254+
time_window_seconds: int | float,
255+
detection_type: str | None,
256+
comparison_delta: int | float | None,
254257
) -> timedelta:
255258
"""
256-
Compute the appropriate SnubaQuery resolution for a given time window,
257-
mirroring the logic in create_alert_rule / update_alert_rule.
259+
Compute the appropriate SnubaQuery resolution for a given time window
260+
(in seconds), mirroring the logic in create_alert_rule / update_alert_rule.
258261
"""
259262
organization = self.context["organization"]
260263

261264
if detection_type == AlertRuleDetectionType.DYNAMIC:
262265
resolution = timedelta(seconds=time_window_seconds)
263266
else:
264-
resolution = get_alert_resolution(time_window_seconds // 60, organization)
267+
resolution = get_alert_resolution(int(time_window_seconds) // 60, organization)
265268
if comparison_delta is not None:
266269
resolution *= DEFAULT_CMP_ALERT_RULE_RESOLUTION_MULTIPLIER
267270

@@ -294,7 +297,7 @@ def get_quota(self) -> DetectorQuota:
294297
return DetectorQuota(has_exceeded=has_exceeded, limit=detector_limit, count=detector_count)
295298

296299
def is_editing_transaction_dataset(
297-
self, snuba_query: SnubaQuery, data_source: SnubaQueryDataSourceType
300+
self, snuba_query: SnubaQuery, data_source: SnubaQueryDataSourceType | dict[str, Any]
298301
) -> bool:
299302
organization = self.context.get("organization")
300303
current_dataset = Dataset(snuba_query.dataset)
@@ -317,7 +320,10 @@ def is_editing_transaction_dataset(
317320
return False
318321

319322
def update_data_source(
320-
self, instance: Detector, data_source: SnubaQueryDataSourceType, seer_updated: bool = False
323+
self,
324+
instance: Detector,
325+
data_source: SnubaQueryDataSourceType | dict[str, Any],
326+
seer_updated: bool = False,
321327
) -> None:
322328
try:
323329
source_instance = DataSource.objects.get(detector=instance)
@@ -334,7 +340,7 @@ def update_data_source(
334340
except SnubaQuery.DoesNotExist:
335341
raise serializers.ValidationError("SnubaQuery not found, can't update")
336342

337-
event_types = SnubaQueryEventType.objects.filter(snuba_query_id=snuba_query.id)
343+
event_types = snuba_query.event_types
338344

339345
if self.is_editing_transaction_dataset(snuba_query, data_source):
340346
raise serializers.ValidationError(
@@ -373,14 +379,14 @@ def update_data_source(
373379

374380
update_snuba_query(
375381
snuba_query=snuba_query,
376-
query_type=data_source.get("query_type", snuba_query.type),
377-
dataset=data_source.get("dataset", snuba_query.dataset),
382+
query_type=data_source.get("query_type", SnubaQuery.Type(snuba_query.type)),
383+
dataset=data_source.get("dataset", Dataset(snuba_query.dataset)),
378384
query=data_source.get("query", snuba_query.query),
379385
aggregate=data_source.get("aggregate", snuba_query.aggregate),
380386
time_window=timedelta(seconds=new_time_window),
381387
resolution=resolution,
382388
environment=data_source.get("environment", snuba_query.environment),
383-
event_types=data_source.get("event_types", [event_type for event_type in event_types]),
389+
event_types=data_source.get("event_types", event_types),
384390
extrapolation_mode=extrapolation_mode,
385391
)
386392

tests/sentry/incidents/endpoints/validators/test_validators.py

Lines changed: 23 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
from typing import Any
12
from unittest import mock
23

34
import orjson
@@ -756,23 +757,26 @@ def test_create_detector_trace_metrics_valid_aggregates(self) -> None:
756757
)
757758

758759

760+
@mock.patch("sentry.incidents.metric_issue_detector.schedule_update_project_config")
759761
class TestMetricAlertsResolution(TestMetricAlertsDetectorValidator):
760-
"""Tests that resolution is computed correctly via get_alert_resolution."""
762+
def _make_data_source(self, **overrides: Any) -> dict[str, Any]:
763+
return {
764+
"queryType": SnubaQuery.Type.ERROR.value,
765+
"dataset": Dataset.Events.value,
766+
"query": "test query",
767+
"aggregate": "count()",
768+
"timeWindow": 3600,
769+
"environment": self.environment.name,
770+
"eventTypes": [SnubaQueryEventType.EventType.ERROR.name.lower()],
771+
**overrides,
772+
}
761773

762-
@mock.patch("sentry.incidents.metric_issue_detector.schedule_update_project_config")
763-
@mock.patch("sentry.workflow_engine.endpoints.validators.base.detector.create_audit_entry")
764774
def test_create_static_small_window_uses_default_resolution(
765-
self, mock_audit: mock.MagicMock, mock_schedule: mock.MagicMock
775+
self, mock_schedule: mock.MagicMock
766776
) -> None:
767-
"""A 10-minute static detector should get 1-minute resolution (default)."""
768777
data = {
769778
**self.valid_data,
770-
"dataSources": [
771-
{
772-
**self.valid_data["dataSources"][0],
773-
"timeWindow": 600, # 10 minutes in seconds
774-
}
775-
],
779+
"dataSources": [self._make_data_source(timeWindow=600)],
776780
}
777781
validator = MetricIssueDetectorValidator(data=data, context=self.context)
778782
assert validator.is_valid(), validator.errors
@@ -783,20 +787,12 @@ def test_create_static_small_window_uses_default_resolution(
783787
expected = get_alert_resolution(10, self.project.organization).total_seconds()
784788
assert snuba_query.resolution == expected
785789

786-
@mock.patch("sentry.incidents.metric_issue_detector.schedule_update_project_config")
787-
@mock.patch("sentry.workflow_engine.endpoints.validators.base.detector.create_audit_entry")
788790
def test_create_static_large_window_uses_scaled_resolution(
789-
self, mock_audit: mock.MagicMock, mock_schedule: mock.MagicMock
791+
self, mock_schedule: mock.MagicMock
790792
) -> None:
791-
"""A 60-minute static detector should get 3-minute resolution."""
792793
data = {
793794
**self.valid_data,
794-
"dataSources": [
795-
{
796-
**self.valid_data["dataSources"][0],
797-
"timeWindow": 3600, # 60 minutes in seconds
798-
}
799-
],
795+
"dataSources": [self._make_data_source(timeWindow=3600)],
800796
}
801797
validator = MetricIssueDetectorValidator(data=data, context=self.context)
802798
assert validator.is_valid(), validator.errors
@@ -809,11 +805,9 @@ def test_create_static_large_window_uses_scaled_resolution(
809805
assert snuba_query.resolution == 180 # 3 minutes in seconds
810806

811807
@mock.patch("sentry.seer.anomaly_detection.store_data_workflow_engine.make_store_data_request")
812-
@mock.patch("sentry.workflow_engine.endpoints.validators.base.detector.create_audit_entry")
813808
def test_create_dynamic_resolution_equals_time_window(
814-
self, mock_audit: mock.MagicMock, mock_seer_request: mock.MagicMock
809+
self, mock_seer_request: mock.MagicMock, mock_schedule: mock.MagicMock
815810
) -> None:
816-
"""Dynamic detectors should use resolution == time_window."""
817811
seer_return_value: StoreDataResponse = {"success": True}
818812
mock_seer_request.return_value = HTTPResponse(orjson.dumps(seer_return_value), status=200)
819813

@@ -822,20 +816,10 @@ def test_create_dynamic_resolution_equals_time_window(
822816
snuba_query = self.get_snuba_query(detector)
823817
assert snuba_query.resolution == snuba_query.time_window
824818

825-
@mock.patch("sentry.incidents.metric_issue_detector.schedule_update_project_config")
826-
@mock.patch("sentry.workflow_engine.endpoints.validators.base.detector.create_audit_entry")
827-
def test_create_percent_resolution_doubled(
828-
self, mock_audit: mock.MagicMock, mock_schedule: mock.MagicMock
829-
) -> None:
830-
"""Percent (comparison) detectors should double the resolution."""
819+
def test_create_percent_resolution_doubled(self, mock_schedule: mock.MagicMock) -> None:
831820
data = {
832821
**self.valid_data,
833-
"dataSources": [
834-
{
835-
**self.valid_data["dataSources"][0],
836-
"timeWindow": 3600, # 60 minutes
837-
}
838-
],
822+
"dataSources": [self._make_data_source(timeWindow=3600)],
839823
"config": {
840824
"thresholdPeriod": 1,
841825
"detectionType": AlertRuleDetectionType.PERCENT.value,
@@ -853,25 +837,15 @@ def test_create_percent_resolution_doubled(
853837
assert snuba_query.resolution == expected
854838
assert snuba_query.resolution == 360 # 3 min * 2 = 6 minutes in seconds
855839

856-
@mock.patch("sentry.incidents.metric_issue_detector.schedule_update_project_config")
857-
@mock.patch("sentry.workflow_engine.endpoints.validators.base.detector.create_audit_entry")
858840
def test_update_time_window_recalculates_resolution(
859-
self, mock_audit: mock.MagicMock, mock_schedule: mock.MagicMock
841+
self, mock_schedule: mock.MagicMock
860842
) -> None:
861-
"""Changing time_window on update should recalculate resolution."""
862843
detector = self.create_static_detector()
863844
snuba_query = self.get_snuba_query(detector)
864-
old_resolution = snuba_query.resolution
865845

866-
# Update to a larger time window (24 hours = 1440 minutes)
867846
update_data = {
868847
**self.valid_data,
869-
"dataSources": [
870-
{
871-
**self.valid_data["dataSources"][0],
872-
"timeWindow": 86400, # 1440 minutes in seconds
873-
}
874-
],
848+
"dataSources": [self._make_data_source(timeWindow=86400)],
875849
}
876850
update_validator = MetricIssueDetectorValidator(
877851
instance=detector, data=update_data, context=self.context, partial=True
@@ -883,28 +857,23 @@ def test_update_time_window_recalculates_resolution(
883857
expected = get_alert_resolution(1440, self.project.organization).total_seconds()
884858
assert snuba_query.resolution == expected
885859
assert snuba_query.resolution == 900 # 15 minutes in seconds
886-
assert snuba_query.resolution != old_resolution
887860

888861
@mock.patch("sentry.seer.anomaly_detection.store_data_workflow_engine.make_store_data_request")
889862
@mock.patch("sentry.seer.anomaly_detection.delete_rule.delete_rule_in_seer")
890-
@mock.patch("sentry.workflow_engine.endpoints.validators.base.detector.create_audit_entry")
891863
def test_update_detection_type_change_recalculates_resolution(
892864
self,
893-
mock_audit: mock.MagicMock,
894865
mock_seer_delete: mock.MagicMock,
895866
mock_seer_store: mock.MagicMock,
867+
mock_schedule: mock.MagicMock,
896868
) -> None:
897-
"""Changing detection_type without data_sources should still recalculate resolution."""
898869
seer_return_value: StoreDataResponse = {"success": True}
899870
mock_seer_store.return_value = HTTPResponse(orjson.dumps(seer_return_value), status=200)
900871
mock_seer_delete.return_value = HTTPResponse(orjson.dumps(seer_return_value), status=200)
901872

902873
detector = self.create_dynamic_detector()
903874
snuba_query = self.get_snuba_query(detector)
904-
# Dynamic: resolution == time_window
905875
assert snuba_query.resolution == snuba_query.time_window
906876

907-
# Change to static without changing data sources
908877
update_data = {
909878
**self.valid_data,
910879
"config": {

0 commit comments

Comments
 (0)