Skip to content

Commit

Permalink
fix: Correct API usage for Crons (#42886)
Browse files Browse the repository at this point in the history
- Remove legacy docstrings and replace with better descriptions
- Missing checkin details endpoint for Hybrid Cloud

Refs #42850
  • Loading branch information
dcramer authored Jan 6, 2023
1 parent ec012b2 commit 8634c54
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 44 deletions.
46 changes: 26 additions & 20 deletions src/sentry/api/endpoints/monitor_checkin_details.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from __future__ import annotations

from django.db import transaction
from django.utils import timezone
from drf_spectacular.utils import extend_schema
Expand Down Expand Up @@ -41,7 +43,15 @@ class MonitorCheckInDetailsEndpoint(Endpoint):

# TODO(dcramer): this code needs shared with other endpoints as its security focused
# TODO(dcramer): this doesnt handle is_global roles
def convert_args(self, request: Request, monitor_id, checkin_id, *args, **kwargs):
def convert_args(
self,
request: Request,
monitor_id,
checkin_id,
organization_slug: str | None = None,
*args,
**kwargs,
):
try:
monitor = Monitor.objects.get(guid=monitor_id)
except Monitor.DoesNotExist:
Expand All @@ -51,6 +61,10 @@ def convert_args(self, request: Request, monitor_id, checkin_id, *args, **kwargs
if project.status != ProjectStatus.VISIBLE:
raise ResourceDoesNotExist

if organization_slug:
if project.organization.slug != organization_slug:
return self.respond_invalid()

if hasattr(request.auth, "project_id") and project.id != request.auth.project_id:
return self.respond(status=400)

Expand Down Expand Up @@ -102,12 +116,7 @@ def convert_args(self, request: Request, monitor_id, checkin_id, *args, **kwargs
)
def get(self, request: Request, project, monitor, checkin) -> Response:
"""
Retrieve a check-in
```````````````````
:pparam string monitor_id: the id of the monitor.
:pparam string checkin_id: the id of the check-in.
:auth: required
Retrieves details for a check-in.
You may use `latest` for the `checkin_id` parameter in order to retrieve
the most recent (by creation date) check-in which is still mutable (not marked as finished).
Expand All @@ -128,26 +137,22 @@ def get(self, request: Request, project, monitor, checkin) -> Response:
request=MonitorCheckInValidator,
responses={
200: inline_sentry_response_serializer(
"MonitorCheckIn2", MonitorCheckInSerializerResponse
"MonitorCheckIn", MonitorCheckInSerializerResponse
),
208: RESPONSE_ALREADY_REPORTED,
400: RESPONSE_BAD_REQUEST,
401: RESPONSE_UNAUTHORIZED,
403: RESPONSE_FORBIDDEN,
404: RESPONSE_NOTFOUND,
},
examples=[
# OpenApiExample()
],
)
def put(self, request: Request, project, monitor, checkin) -> Response:
"""
Update a check-in
`````````````````
Updates a check-in.
Once a check-in is finished (indicated via an `ok` or `error` status) it can no longer be changed.
:pparam string monitor_id: the id of the monitor.
:pparam string checkin_id: the id of the check-in.
:auth: required
If you simply wish to update that the task is still running, you can simply send an empty payload.
You may use `latest` for the `checkin_id` parameter in order to retrieve
the most recent (by creation date) check-in which is still mutable (not marked as finished).
Expand All @@ -165,15 +170,16 @@ def put(self, request: Request, project, monitor, checkin) -> Response:

current_datetime = timezone.now()
params = {"date_updated": current_datetime}
if "status" in result:
params["status"] = getattr(CheckInStatus, result["status"].upper())

if "duration" in result:
params["duration"] = result["duration"]
else:
# if a duration is not defined and we're at a finish state, calculate one
elif params.get("status", checkin.status) in CheckInStatus.FINISHED_VALUES:
duration = int((current_datetime - checkin.date_added).total_seconds() * 1000)
params["duration"] = duration

if "status" in result:
params["status"] = getattr(CheckInStatus, result["status"].upper())

with transaction.atomic():
checkin.update(**params)
if checkin.status == CheckInStatus.ERROR:
Expand Down
15 changes: 6 additions & 9 deletions src/sentry/api/endpoints/monitor_checkins.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,7 @@ def get(
self, request: Request, project, monitor, organization_slug: str | None = None
) -> Response:
"""
Retrieve check-ins for a monitor
````````````````````````````````
:pparam string monitor_id: the id of the monitor.
:auth: required
Retrieve a list of check-ins for a monitor
"""
# we don't allow read permission with DSNs
if isinstance(request.auth, ProjectKey):
Expand Down Expand Up @@ -100,11 +96,12 @@ def post(
self, request: Request, project, monitor, organization_slug: str | None = None
) -> Response:
"""
Create a new check-in for a monitor
```````````````````````````````````
Creates a new check-in for a monitor.
If `status` is not present, it will be assumed that the check-in is starting, and be marked as `in_progress`.
:pparam string monitor_id: the id of the monitor.
:auth: required
To achieve a ping-like behavior, you can simply define `status` and optionally `duration` and
this check-in will be automatically marked as finished.
Note: If a DSN is utilized for authentication, the response will be limited in details.
"""
Expand Down
19 changes: 4 additions & 15 deletions src/sentry/api/endpoints/monitor_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,7 @@ def get(
self, request: Request, project, monitor, organization_slug: str | None = None
) -> Response:
"""
Retrieve a monitor
``````````````````
:pparam string monitor_id: the id of the monitor.
:auth: required
Retrieves details for a monitor.
"""
if organization_slug:
if project.organization.slug != organization_slug:
Expand All @@ -74,11 +70,7 @@ def put(
self, request: Request, project, monitor, organization_slug: str | None = None
) -> Response:
"""
Update a monitor
````````````````
:pparam string monitor_id: the id of the monitor.
:auth: required
Update a monitor.
"""
if organization_slug:
if project.organization.slug != organization_slug:
Expand Down Expand Up @@ -145,12 +137,9 @@ def delete(
self, request: Request, project, monitor, organization_slug: str | None = None
) -> Response:
"""
Delete a monitor
````````````````
:pparam string monitor_id: the id of the monitor.
:auth: required
Delete a monitor.
"""

if organization_slug:
if project.organization.slug != organization_slug:
return self.respond_invalid()
Expand Down
5 changes: 5 additions & 0 deletions src/sentry/api/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -713,6 +713,11 @@
MonitorCheckInsEndpoint.as_view(),
name="sentry-api-0-monitor-check-in-index-with-org",
),
url(
r"^(?P<monitor_id>[^\/]+)/checkins/(?P<checkin_id>[^\/]+)/$",
MonitorCheckInDetailsEndpoint.as_view(),
name="sentry-api-0-monitor-check-in-details-with-org",
),
url(
r"^(?P<monitor_id>[^\/]+)/stats/$",
MonitorStatsEndpoint.as_view(),
Expand Down
22 changes: 22 additions & 0 deletions tests/sentry/api/endpoints/test_monitor_checkin_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,28 @@ def setUp(self):
super().setUp()
self.login_as(self.user)

def test_noop_in_progerss(self):
monitor = Monitor.objects.create(
organization_id=self.organization.id,
project_id=self.project.id,
next_checkin=timezone.now() - timedelta(minutes=1),
type=MonitorType.CRON_JOB,
config={"schedule": "* * * * *"},
date_added=timezone.now() - timedelta(minutes=1),
)
checkin = MonitorCheckIn.objects.create(
monitor=monitor,
project_id=self.project.id,
date_added=monitor.date_added,
status=CheckInStatus.IN_PROGRESS,
)

self.get_success_response(monitor.guid, checkin.guid)

checkin = MonitorCheckIn.objects.get(id=checkin.id)
assert checkin.status == CheckInStatus.IN_PROGRESS
assert checkin.date_updated > checkin.date_added

def test_passing(self):
monitor = Monitor.objects.create(
organization_id=self.organization.id,
Expand Down

0 comments on commit 8634c54

Please sign in to comment.