Skip to content

Commit 518a1a6

Browse files
committed
typing fix
1 parent 9faa983 commit 518a1a6

File tree

4 files changed

+44
-88
lines changed

4 files changed

+44
-88
lines changed

src/sentry/incidents/logic.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -928,12 +928,12 @@ def update_alert_rule(
928928
updated_fields["sensitivity"] = None
929929
updated_fields["seasonality"] = None
930930
elif detection_type == AlertRuleDetectionType.DYNAMIC:
931-
# NOTE: we set seasonality for EA
932931
if time_window is not None:
933932
updated_query_fields["resolution"] = timedelta(minutes=time_window)
934933
else:
935934
# snuba_query.time_window is already in seconds
936935
updated_query_fields["resolution"] = timedelta(seconds=snuba_query.time_window)
936+
# NOTE: we set seasonality for EA
937937
updated_fields["seasonality"] = AlertRuleSeasonality.AUTO
938938
updated_fields["comparison_delta"] = None
939939
if (

src/sentry/incidents/metric_issue_detector.py

Lines changed: 15 additions & 9 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

@@ -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,
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

src/sentry/workflow_engine/types.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ class DataConditionType(TypedDict):
392392

393393

394394
# TODO - Move this to snuba module
395-
class SnubaQueryDataSourceType(TypedDict):
395+
class SnubaQueryDataSourceType(TypedDict, total=False):
396396
query_type: SnubaQuery.Type
397397
dataset: Dataset
398398
query: str

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

Lines changed: 27 additions & 77 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,28 @@ def test_create_detector_trace_metrics_valid_aggregates(self) -> None:
756757
)
757758

758759

760+
# No-op Seer and relay interactions — these tests only verify resolution values.
761+
@mock.patch("sentry.incidents.metric_issue_detector.delete_data_in_seer_for_detector")
762+
@mock.patch("sentry.incidents.metric_issue_detector.send_new_detector_data")
763+
@mock.patch("sentry.incidents.metric_issue_detector.update_detector_data")
764+
@mock.patch("sentry.incidents.metric_issue_detector.schedule_update_project_config")
759765
class TestMetricAlertsResolution(TestMetricAlertsDetectorValidator):
760-
"""Tests that resolution is computed correctly via get_alert_resolution."""
766+
def _make_data_source(self, **overrides: Any) -> dict[str, Any]:
767+
return {
768+
"queryType": SnubaQuery.Type.ERROR.value,
769+
"dataset": Dataset.Events.value,
770+
"query": "test query",
771+
"aggregate": "count()",
772+
"timeWindow": 3600,
773+
"environment": self.environment.name,
774+
"eventTypes": [SnubaQueryEventType.EventType.ERROR.name.lower()],
775+
**overrides,
776+
}
761777

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")
764-
def test_create_static_small_window_uses_default_resolution(
765-
self, mock_audit: mock.MagicMock, mock_schedule: mock.MagicMock
766-
) -> None:
767-
"""A 10-minute static detector should get 1-minute resolution (default)."""
778+
def test_create_static_small_window_uses_default_resolution(self, *mocks: Any) -> None:
768779
data = {
769780
**self.valid_data,
770-
"dataSources": [
771-
{
772-
**self.valid_data["dataSources"][0],
773-
"timeWindow": 600, # 10 minutes in seconds
774-
}
775-
],
781+
"dataSources": [self._make_data_source(timeWindow=600)],
776782
}
777783
validator = MetricIssueDetectorValidator(data=data, context=self.context)
778784
assert validator.is_valid(), validator.errors
@@ -783,20 +789,10 @@ def test_create_static_small_window_uses_default_resolution(
783789
expected = get_alert_resolution(10, self.project.organization).total_seconds()
784790
assert snuba_query.resolution == expected
785791

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")
788-
def test_create_static_large_window_uses_scaled_resolution(
789-
self, mock_audit: mock.MagicMock, mock_schedule: mock.MagicMock
790-
) -> None:
791-
"""A 60-minute static detector should get 3-minute resolution."""
792+
def test_create_static_large_window_uses_scaled_resolution(self, *mocks: Any) -> None:
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
@@ -808,34 +804,16 @@ def test_create_static_large_window_uses_scaled_resolution(
808804
assert snuba_query.resolution == expected
809805
assert snuba_query.resolution == 180 # 3 minutes in seconds
810806

811-
@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")
813-
def test_create_dynamic_resolution_equals_time_window(
814-
self, mock_audit: mock.MagicMock, mock_seer_request: mock.MagicMock
815-
) -> None:
816-
"""Dynamic detectors should use resolution == time_window."""
817-
seer_return_value: StoreDataResponse = {"success": True}
818-
mock_seer_request.return_value = HTTPResponse(orjson.dumps(seer_return_value), status=200)
819-
807+
def test_create_dynamic_resolution_equals_time_window(self, *mocks: Any) -> None:
820808
detector = self.create_dynamic_detector()
821809

822810
snuba_query = self.get_snuba_query(detector)
823811
assert snuba_query.resolution == snuba_query.time_window
824812

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."""
813+
def test_create_percent_resolution_doubled(self, *mocks: Any) -> None:
831814
data = {
832815
**self.valid_data,
833-
"dataSources": [
834-
{
835-
**self.valid_data["dataSources"][0],
836-
"timeWindow": 3600, # 60 minutes
837-
}
838-
],
816+
"dataSources": [self._make_data_source(timeWindow=3600)],
839817
"config": {
840818
"thresholdPeriod": 1,
841819
"detectionType": AlertRuleDetectionType.PERCENT.value,
@@ -853,25 +831,13 @@ def test_create_percent_resolution_doubled(
853831
assert snuba_query.resolution == expected
854832
assert snuba_query.resolution == 360 # 3 min * 2 = 6 minutes in seconds
855833

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")
858-
def test_update_time_window_recalculates_resolution(
859-
self, mock_audit: mock.MagicMock, mock_schedule: mock.MagicMock
860-
) -> None:
861-
"""Changing time_window on update should recalculate resolution."""
834+
def test_update_time_window_recalculates_resolution(self, *mocks: Any) -> None:
862835
detector = self.create_static_detector()
863836
snuba_query = self.get_snuba_query(detector)
864-
old_resolution = snuba_query.resolution
865837

866-
# Update to a larger time window (24 hours = 1440 minutes)
867838
update_data = {
868839
**self.valid_data,
869-
"dataSources": [
870-
{
871-
**self.valid_data["dataSources"][0],
872-
"timeWindow": 86400, # 1440 minutes in seconds
873-
}
874-
],
840+
"dataSources": [self._make_data_source(timeWindow=86400)],
875841
}
876842
update_validator = MetricIssueDetectorValidator(
877843
instance=detector, data=update_data, context=self.context, partial=True
@@ -883,28 +849,12 @@ def test_update_time_window_recalculates_resolution(
883849
expected = get_alert_resolution(1440, self.project.organization).total_seconds()
884850
assert snuba_query.resolution == expected
885851
assert snuba_query.resolution == 900 # 15 minutes in seconds
886-
assert snuba_query.resolution != old_resolution
887-
888-
@mock.patch("sentry.seer.anomaly_detection.store_data_workflow_engine.make_store_data_request")
889-
@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")
891-
def test_update_detection_type_change_recalculates_resolution(
892-
self,
893-
mock_audit: mock.MagicMock,
894-
mock_seer_delete: mock.MagicMock,
895-
mock_seer_store: mock.MagicMock,
896-
) -> None:
897-
"""Changing detection_type without data_sources should still recalculate resolution."""
898-
seer_return_value: StoreDataResponse = {"success": True}
899-
mock_seer_store.return_value = HTTPResponse(orjson.dumps(seer_return_value), status=200)
900-
mock_seer_delete.return_value = HTTPResponse(orjson.dumps(seer_return_value), status=200)
901852

853+
def test_update_detection_type_change_recalculates_resolution(self, *mocks: Any) -> None:
902854
detector = self.create_dynamic_detector()
903855
snuba_query = self.get_snuba_query(detector)
904-
# Dynamic: resolution == time_window
905856
assert snuba_query.resolution == snuba_query.time_window
906857

907-
# Change to static without changing data sources
908858
update_data = {
909859
**self.valid_data,
910860
"config": {

0 commit comments

Comments
 (0)