From d95bd864ec6bbf929793476297d99322b62b46e1 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Wed, 20 Aug 2025 13:50:48 -0700 Subject: [PATCH 1/2] type pagerduty module --- pyproject.toml | 2 +- src/sentry/integrations/base.py | 2 +- .../integrations/pagerduty/actions/form.py | 17 +++---- .../pagerduty/actions/notification.py | 26 ++++++++--- src/sentry/integrations/pagerduty/client.py | 2 +- .../integrations/pagerduty/integration.py | 45 ++++++++++++------- src/sentry/integrations/pagerduty/metrics.py | 2 +- src/sentry/integrations/pagerduty/utils.py | 2 +- src/sentry/utils/forms.py | 9 +++- 9 files changed, 71 insertions(+), 36 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 2dee5af8731868..e0780d3ecb4ffd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -307,7 +307,6 @@ module = [ "sentry.api.endpoints.organization_releases", "sentry.api.paginator", "sentry.db.postgres.base", - "sentry.integrations.pagerduty.actions.form", "sentry.integrations.slack.message_builder.notifications.issues", "sentry.integrations.slack.webhooks.event", "sentry.issues.search", @@ -427,6 +426,7 @@ module = [ "sentry.integrations.jira_server.actions.*", "sentry.integrations.jira_server.utils.*", "sentry.integrations.models.integration_feature", + "sentry.integrations.pagerduty.*", "sentry.integrations.project_management.*", "sentry.integrations.repository.*", "sentry.integrations.services.*", diff --git a/src/sentry/integrations/base.py b/src/sentry/integrations/base.py index 0a511c7df7bf38..875ab6c088a508 100644 --- a/src/sentry/integrations/base.py +++ b/src/sentry/integrations/base.py @@ -414,7 +414,7 @@ def update_organization_config(self, data: MutableMapping[str, Any]) -> None: if org_integration is not None: self.org_integration = org_integration - def get_config_data(self) -> Mapping[str, str]: + def get_config_data(self) -> Mapping[str, Any]: if not self.org_integration: return {} return self.org_integration.config diff --git a/src/sentry/integrations/pagerduty/actions/form.py b/src/sentry/integrations/pagerduty/actions/form.py index a6f468f7539d40..2b16c8607de43a 100644 --- a/src/sentry/integrations/pagerduty/actions/form.py +++ b/src/sentry/integrations/pagerduty/actions/form.py @@ -10,6 +10,7 @@ from sentry.integrations.pagerduty.metrics import record_event from sentry.integrations.services.integration import integration_service from sentry.integrations.types import ExternalProviders +from sentry.utils.forms import get_field_choices, set_field_choices def _validate_int_field(field: str, cleaned_data: Mapping[str, Any]) -> int | None: @@ -29,7 +30,7 @@ class PagerDutyNotifyServiceForm(forms.Form): account = forms.ChoiceField(choices=(), widget=forms.Select()) service = forms.ChoiceField(required=False, choices=(), widget=forms.Select()) - def __init__(self, *args, **kwargs): + def __init__(self, *args: Any, **kwargs: Any) -> None: integrations = [(i.id, i.name) for i in kwargs.pop("integrations")] services = kwargs.pop("services") @@ -37,20 +38,18 @@ def __init__(self, *args, **kwargs): if integrations: self.fields["account"].initial = integrations[0][0] - self.fields["account"].choices = integrations - self.fields["account"].widget.choices = self.fields["account"].choices + set_field_choices(self.fields["account"], integrations) if services: self.fields["service"].initial = services[0][0] - self.fields["service"].choices = services - self.fields["service"].widget.choices = self.fields["service"].choices + set_field_choices(self.fields["service"], services) def _validate_service(self, service_id: int, integration_id: int) -> None: with record_event(OnCallInteractionType.VALIDATE_SERVICE).capture() as lifecycle: params = { - "account": dict(self.fields["account"].choices).get(integration_id), - "service": dict(self.fields["service"].choices).get(service_id), + "account": dict(get_field_choices(self.fields["account"])).get(integration_id), + "service": dict(get_field_choices(self.fields["service"])).get(service_id), } org_integrations = integration_service.get_organization_integrations( @@ -77,11 +76,13 @@ def _validate_service(self, service_id: int, integration_id: int) -> None: def clean(self) -> dict[str, Any] | None: cleaned_data = super().clean() + if cleaned_data is None: + return cleaned_data integration_id = _validate_int_field("account", cleaned_data) service_id = _validate_int_field("service", cleaned_data) - if service_id: + if service_id and integration_id: self._validate_service(service_id, integration_id) return cleaned_data diff --git a/src/sentry/integrations/pagerduty/actions/notification.py b/src/sentry/integrations/pagerduty/actions/notification.py index 18fca6d7b07f04..9f19533a7505e7 100644 --- a/src/sentry/integrations/pagerduty/actions/notification.py +++ b/src/sentry/integrations/pagerduty/actions/notification.py @@ -1,8 +1,8 @@ from __future__ import annotations import logging -from collections.abc import Sequence -from typing import cast +from collections.abc import Generator, Sequence +from typing import Any, TypedDict, cast import sentry_sdk @@ -16,12 +16,22 @@ from sentry.integrations.types import IntegrationProviderSlug from sentry.models.rule import Rule from sentry.rules.actions import IntegrationEventAction +from sentry.rules.base import CallbackFuture +from sentry.services.eventstore.models import GroupEvent from sentry.shared_integrations.exceptions import ApiError +from sentry.types.rules import RuleFuture from sentry.utils.strings import truncatechars logger = logging.getLogger("sentry.integrations.pagerduty") +class PagerDutyService(TypedDict): + id: int + integration_key: str + service_name: str + integration_id: int + + class PagerDutyNotifyServiceAction(IntegrationEventAction): id = "sentry.integrations.pagerduty.notify_action.PagerDutyNotifyServiceAction" label = "Send a notification to PagerDuty account {account} and service {service} with {severity} severity" @@ -29,7 +39,7 @@ class PagerDutyNotifyServiceAction(IntegrationEventAction): provider = IntegrationProviderSlug.PAGERDUTY.value integration_key = "account" - def __init__(self, *args, **kwargs): + def __init__(self, *args: Any, **kwargs: Any) -> None: super().__init__(*args, **kwargs) self.form_fields = { "account": { @@ -49,7 +59,7 @@ def __init__(self, *args, **kwargs): }, } - def _get_service(self): + def _get_service(self) -> PagerDutyService | None: oi = self.get_organization_integration() if not oi: return None @@ -58,7 +68,9 @@ def _get_service(self): return pds return None - def after(self, event, notification_uuid: str | None = None): + def after( + self, event: GroupEvent, notification_uuid: str | None = None + ) -> Generator[CallbackFuture]: integration = self.get_integration() log_context = { "organization_id": self.project.organization_id, @@ -79,7 +91,7 @@ def after(self, event, notification_uuid: str | None = None): PagerdutySeverity, self.get_option("severity", default=PAGERDUTY_DEFAULT_SEVERITY) ) - def send_notification(event, futures): + def send_notification(event: GroupEvent, futures: Sequence[RuleFuture]) -> None: installation = integration.get_installation(self.project.organization_id) try: client = installation.get_keyring_client(self.get_option("service")) @@ -147,7 +159,7 @@ def get_services(self) -> Sequence[tuple[int, str]]: for v in oi.config.get("pagerduty_services", []) ] - def render_label(self): + def render_label(self) -> str: s = self._get_service() if s: service_name = s["service_name"] diff --git a/src/sentry/integrations/pagerduty/client.py b/src/sentry/integrations/pagerduty/client.py index bc024bac79a301..b0988ded24e712 100644 --- a/src/sentry/integrations/pagerduty/client.py +++ b/src/sentry/integrations/pagerduty/client.py @@ -46,7 +46,7 @@ def request(self, *args: Any, **kwargs: Any) -> Any: kwargs.setdefault("headers", {"Content-Type": "application/json"}) return self._request(*args, **kwargs) - def send_trigger(self, data: PagerDutyEventPayload): + def send_trigger(self, data: PagerDutyEventPayload) -> Any: with record_event(OnCallInteractionType.CREATE).capture(): return self.post("/", data=data) diff --git a/src/sentry/integrations/pagerduty/integration.py b/src/sentry/integrations/pagerduty/integration.py index b6dadfdc99aca5..783c7972f53d5b 100644 --- a/src/sentry/integrations/pagerduty/integration.py +++ b/src/sentry/integrations/pagerduty/integration.py @@ -1,8 +1,8 @@ from __future__ import annotations import logging -from collections.abc import Mapping, Sequence -from typing import Any +from collections.abc import Mapping, MutableMapping, Sequence +from typing import Any, TypedDict import orjson from django.db import router, transaction @@ -73,6 +73,23 @@ ) +class PagerDutyOrganizationConfig(TypedDict): + name: str + type: str + label: str + help: str + addButtonText: str + columnLabels: dict[str, str] + columnKeys: list[str] + confirmDeleteMessage: str + + +class PagerDutyServiceConfig(TypedDict): + service: str + integration_key: str + id: int + + class PagerDutyIntegration(IntegrationInstallation): def get_keyring_client(self, keyid: int | str) -> PagerDutyClient: org_integration = self.org_integration @@ -89,11 +106,11 @@ def get_keyring_client(self, keyid: int | str) -> PagerDutyClient: integration_id=org_integration.integration_id, integration_key=integration_key ) - def get_client(self): + def get_client(self) -> None: raise NotImplementedError("Use get_keyring_client instead.") - def get_organization_config(self): - fields = [ + def get_organization_config(self) -> list[PagerDutyOrganizationConfig]: + return [ { "name": "service_table", "type": "table", @@ -106,9 +123,7 @@ def get_organization_config(self): } ] - return fields - - def update_organization_config(self, data): + def update_organization_config(self, data: MutableMapping[str, Any]) -> None: if "service_table" in data: service_rows = data["service_table"] # validate fields @@ -149,15 +164,15 @@ def update_organization_config(self, data): key = row["integration_key"] add_service(oi, integration_key=key, service_name=service_name) - def get_config_data(self): + def get_config_data(self) -> Mapping[str, list[PagerDutyServiceConfig]]: service_list = [] for s in self.services: service_list.append( - { - "service": s["service_name"], - "integration_key": s["integration_key"], - "id": s["id"], - } + PagerDutyServiceConfig( + service=s["service_name"], + integration_key=s["integration_key"], + id=s["id"], + ) ) return {"service_table": service_list} @@ -220,7 +235,7 @@ def build_integration(self, state: Mapping[str, Any]) -> IntegrationData: class PagerDutyInstallationRedirect: - def get_app_url(self, account_name=None): + def get_app_url(self, account_name: str | None = None) -> str: if not account_name: account_name = "app" diff --git a/src/sentry/integrations/pagerduty/metrics.py b/src/sentry/integrations/pagerduty/metrics.py index 8f82ec36285ebe..ed92756f4bb461 100644 --- a/src/sentry/integrations/pagerduty/metrics.py +++ b/src/sentry/integrations/pagerduty/metrics.py @@ -2,5 +2,5 @@ from sentry.integrations.on_call.spec import PagerDutyOnCallSpec -def record_event(event: OnCallInteractionType): +def record_event(event: OnCallInteractionType) -> OnCallInteractionEvent: return OnCallInteractionEvent(event, PagerDutyOnCallSpec()) diff --git a/src/sentry/integrations/pagerduty/utils.py b/src/sentry/integrations/pagerduty/utils.py index 4ecbcea58dff96..4f4c955c7f18df 100644 --- a/src/sentry/integrations/pagerduty/utils.py +++ b/src/sentry/integrations/pagerduty/utils.py @@ -95,7 +95,7 @@ def build_incident_attachment( alert_context: AlertContext, metric_issue_context: MetricIssueContext, organization: Organization, - integration_key, + integration_key: str, notification_uuid: str | None = None, ) -> dict[str, Any]: diff --git a/src/sentry/utils/forms.py b/src/sentry/utils/forms.py index d6bcd62e6cb22b..f1586a9784a80a 100644 --- a/src/sentry/utils/forms.py +++ b/src/sentry/utils/forms.py @@ -1,7 +1,7 @@ from __future__ import annotations from collections.abc import Sequence -from typing import TYPE_CHECKING, NotRequired, TypedDict +from typing import TYPE_CHECKING, Any, NotRequired, TypedDict from django import forms from django.forms import ChoiceField @@ -61,3 +61,10 @@ def set_field_choices(field: forms.Field, choices: Sequence[tuple[object, object raise TypeError(f"expected ChoiceField, got {field!r}") field.choices = choices field.widget.choices = choices + + +def get_field_choices(field: forms.Field) -> Any: + """workaround for typeddjango/django-stubs#1514 & #1858""" + if not isinstance(field, ChoiceField): + raise TypeError(f"expected ChoiceField, got {field!r}") + return field.choices From bba3085b757b2cee2adb3dafd06fca33c9efdd95 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Thu, 21 Aug 2025 10:12:54 -0700 Subject: [PATCH 2/2] use properties to set instead --- .../integrations/pagerduty/actions/form.py | 22 +++++++++---------- src/sentry/utils/forms.py | 9 +------- 2 files changed, 12 insertions(+), 19 deletions(-) diff --git a/src/sentry/integrations/pagerduty/actions/form.py b/src/sentry/integrations/pagerduty/actions/form.py index 2b16c8607de43a..ee7d6b25cf1499 100644 --- a/src/sentry/integrations/pagerduty/actions/form.py +++ b/src/sentry/integrations/pagerduty/actions/form.py @@ -10,7 +10,7 @@ from sentry.integrations.pagerduty.metrics import record_event from sentry.integrations.services.integration import integration_service from sentry.integrations.types import ExternalProviders -from sentry.utils.forms import get_field_choices, set_field_choices +from sentry.utils.forms import set_field_choices def _validate_int_field(field: str, cleaned_data: Mapping[str, Any]) -> int | None: @@ -31,25 +31,25 @@ class PagerDutyNotifyServiceForm(forms.Form): service = forms.ChoiceField(required=False, choices=(), widget=forms.Select()) def __init__(self, *args: Any, **kwargs: Any) -> None: - integrations = [(i.id, i.name) for i in kwargs.pop("integrations")] - services = kwargs.pop("services") + self._integrations = [(i.id, i.name) for i in kwargs.pop("integrations")] + self._services = kwargs.pop("services") super().__init__(*args, **kwargs) - if integrations: - self.fields["account"].initial = integrations[0][0] + if self._integrations: + self.fields["account"].initial = self._integrations[0][0] - set_field_choices(self.fields["account"], integrations) + set_field_choices(self.fields["account"], self._integrations) - if services: - self.fields["service"].initial = services[0][0] + if self._services: + self.fields["service"].initial = self._services[0][0] - set_field_choices(self.fields["service"], services) + set_field_choices(self.fields["service"], self._services) def _validate_service(self, service_id: int, integration_id: int) -> None: with record_event(OnCallInteractionType.VALIDATE_SERVICE).capture() as lifecycle: params = { - "account": dict(get_field_choices(self.fields["account"])).get(integration_id), - "service": dict(get_field_choices(self.fields["service"])).get(service_id), + "account": dict(self._integrations).get(integration_id), + "service": dict(self._services).get(service_id), } org_integrations = integration_service.get_organization_integrations( diff --git a/src/sentry/utils/forms.py b/src/sentry/utils/forms.py index f1586a9784a80a..d6bcd62e6cb22b 100644 --- a/src/sentry/utils/forms.py +++ b/src/sentry/utils/forms.py @@ -1,7 +1,7 @@ from __future__ import annotations from collections.abc import Sequence -from typing import TYPE_CHECKING, Any, NotRequired, TypedDict +from typing import TYPE_CHECKING, NotRequired, TypedDict from django import forms from django.forms import ChoiceField @@ -61,10 +61,3 @@ def set_field_choices(field: forms.Field, choices: Sequence[tuple[object, object raise TypeError(f"expected ChoiceField, got {field!r}") field.choices = choices field.widget.choices = choices - - -def get_field_choices(field: forms.Field) -> Any: - """workaround for typeddjango/django-stubs#1514 & #1858""" - if not isinstance(field, ChoiceField): - raise TypeError(f"expected ChoiceField, got {field!r}") - return field.choices