Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(typing): Make monitor_config a TypedDict #2931

Merged
merged 15 commits into from
Apr 10, 2024
Merged
34 changes: 34 additions & 0 deletions sentry_sdk/_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,3 +178,37 @@

BucketKey = Tuple[MetricType, str, MeasurementUnit, MetricTagsInternal]
MetricMetaKey = Tuple[MetricType, str, MeasurementUnit]

MonitorConfigScheduleType = Literal["crontab", "interval"]
MonitorConfigScheduleUnit = Literal[
"year",
"month",
"week",
"day",
"hour",
"minute",
"second", # not supported in Sentry and will result in a warning
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second-level precision can still be provided by the user so this needs to be part of the type

]

MonitorConfigSchedule = TypedDict(
"MonitorConfigSchedule",
{
"type": MonitorConfigScheduleType,
"value": Union[int, str],
"unit": MonitorConfigScheduleUnit,
},
total=False,
)

MonitorConfig = TypedDict(
"MonitorConfig",
{
"schedule": MonitorConfigSchedule,
"timezone": str,
"checkin_margin": int,
"max_runtime": int,
"failure_issue_threshold": int,
"recovery_threshold": int,
},
total=False,
)
28 changes: 14 additions & 14 deletions sentry_sdk/crons/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,18 @@


if TYPE_CHECKING:
from typing import Any, Dict, Optional
from sentry_sdk._types import Event
from typing import Optional
from sentry_sdk._types import Event, MonitorConfig

Check warning on line 9 in sentry_sdk/crons/api.py

View check run for this annotation

Codecov / codecov/patch

sentry_sdk/crons/api.py#L8-L9

Added lines #L8 - L9 were not covered by tests


def _create_check_in_event(
monitor_slug=None,
check_in_id=None,
status=None,
duration_s=None,
monitor_config=None,
monitor_slug=None, # type: Optional[str]
check_in_id=None, # type: Optional[str]
status=None, # type: Optional[str]
duration_s=None, # type: Optional[float]
monitor_config=None, # type: Optional[MonitorConfig]
):
# type: (Optional[str], Optional[str], Optional[str], Optional[float], Optional[Dict[str, Any]]) -> Event
# type: (...) -> Event
options = Hub.current.client.options if Hub.current.client else {}
check_in_id = check_in_id or uuid.uuid4().hex # type: str

Expand All @@ -37,13 +37,13 @@


def capture_checkin(
monitor_slug=None,
check_in_id=None,
status=None,
duration=None,
monitor_config=None,
monitor_slug=None, # type: Optional[str]
check_in_id=None, # type: Optional[str]
status=None, # type: Optional[str]
duration=None, # type: Optional[float]
monitor_config=None, # type: Optional[MonitorConfig]
):
# type: (Optional[str], Optional[str], Optional[str], Optional[float], Optional[Dict[str, Any]]) -> str
# type: (...) -> str
check_in_event = _create_check_in_event(
monitor_slug=monitor_slug,
check_in_id=check_in_id,
Expand Down
5 changes: 3 additions & 2 deletions sentry_sdk/crons/decorator.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@
from sentry_sdk.utils import now

if TYPE_CHECKING:
from typing import Any, Optional, Type
from typing import Optional, Type

Check warning on line 8 in sentry_sdk/crons/decorator.py

View check run for this annotation

Codecov / codecov/patch

sentry_sdk/crons/decorator.py#L8

Added line #L8 was not covered by tests
from types import TracebackType
from sentry_sdk._types import MonitorConfig

Check warning on line 10 in sentry_sdk/crons/decorator.py

View check run for this annotation

Codecov / codecov/patch

sentry_sdk/crons/decorator.py#L10

Added line #L10 was not covered by tests

if PY2:
from sentry_sdk.crons._decorator_py2 import MonitorMixin
Expand Down Expand Up @@ -48,7 +49,7 @@
"""

def __init__(self, monitor_slug=None, monitor_config=None):
# type: (Optional[str], Optional[dict[str, Any]]) -> None
# type: (Optional[str], Optional[MonitorConfig]) -> None
self.monitor_slug = monitor_slug
self.monitor_config = monitor_config

Expand Down
27 changes: 20 additions & 7 deletions sentry_sdk/integrations/celery.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
import sys
import time

try:
from typing import cast
except ImportError:

Check warning on line 8 in sentry_sdk/integrations/celery.py

View check run for this annotation

Codecov / codecov/patch

sentry_sdk/integrations/celery.py#L8

Added line #L8 was not covered by tests
cast = lambda _, o: o

from sentry_sdk.api import continue_trace
from sentry_sdk.consts import OP
from sentry_sdk._compat import reraise
Expand Down Expand Up @@ -31,7 +36,15 @@
from typing import Union

from sentry_sdk.tracing import Span
from sentry_sdk._types import EventProcessor, Event, Hint, ExcInfo
from sentry_sdk._types import (

Check warning on line 39 in sentry_sdk/integrations/celery.py

View check run for this annotation

Codecov / codecov/patch

sentry_sdk/integrations/celery.py#L39

Added line #L39 was not covered by tests
EventProcessor,
Event,
Hint,
ExcInfo,
MonitorConfig,
MonitorConfigScheduleType,
MonitorConfigScheduleUnit,
)

F = TypeVar("F", bound=Callable[..., Any])

Expand Down Expand Up @@ -416,7 +429,7 @@


def _get_humanized_interval(seconds):
# type: (float) -> Tuple[int, str]
# type: (float) -> Tuple[int, MonitorConfigScheduleUnit]
TIME_UNITS = ( # noqa: N806
("day", 60 * 60 * 24.0),
("hour", 60 * 60.0),
Expand All @@ -427,17 +440,17 @@
for unit, divider in TIME_UNITS:
if seconds >= divider:
interval = int(seconds / divider)
return (interval, unit)
return (interval, cast("MonitorConfigScheduleUnit", unit))
Copy link
Contributor Author

@sentrivana sentrivana Apr 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mypy is not smart enough to realize that unit comes from TIME_UNITS which is an immutable tuple of tuples where the first element is a subset of MonitorConfigScheduleUnit. Trying to add a type comment to line 437 to tell mypy the precise type of TIME_UNITS doesn't work either. So we cast.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@szokeasaurusrex Do you think this cast is okay like this (aliased to lambda _, x: x at runtime) or should the cast itself be hidden behind TYPE_CHECKING here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cast probably needs to be behind TYPE_CHECKING to maintain compatibility with Python 2.x.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nvm, looks like you handled the compatibility concern by importing as lambda _, x: x when the import fails. Although, worth noting, that I think this lambda only gets activated on Python 2.x. In runtime on Python 3.5+, we still call cast from typing, but that is fine because it is a no-op that returns the second argument at runtime.


return (int(seconds), "second")


def _get_monitor_config(celery_schedule, app, monitor_name):
# type: (Any, Celery, str) -> Dict[str, Any]
monitor_config = {} # type: Dict[str, Any]
schedule_type = None # type: Optional[str]
# type: (Any, Celery, str) -> MonitorConfig
monitor_config = {} # type: MonitorConfig
schedule_type = None # type: Optional[MonitorConfigScheduleType]
schedule_value = None # type: Optional[Union[str, int]]
schedule_unit = None # type: Optional[str]
schedule_unit = None # type: Optional[MonitorConfigScheduleUnit]

if isinstance(celery_schedule, crontab):
schedule_type = "crontab"
Expand Down