Skip to content

feat(python): Improve crons docs, add upserts#9597

Merged
sentrivana merged 6 commits intomasterfrom
ivana/python-cron-upsert
Apr 10, 2024
Merged

feat(python): Improve crons docs, add upserts#9597
sentrivana merged 6 commits intomasterfrom
ivana/python-cron-upsert

Conversation

@sentrivana
Copy link
Contributor

@sentrivana sentrivana commented Apr 2, 2024

Pre-merge checklist

If you work at Sentry, you're able to merge your own PR without review, but please don't unless there's a good reason.

  • Checked Vercel preview for correctness, including links
  • PR was reviewed and approved by any necessary SMEs
  • PR was reviewed and approved by a member of the Sentry docs team

Description of changes

With getsentry/sentry-python#2929 we'll be adding cron upserts to the Python SDK.

Also documenting manual check-ins which were missing.

Direct link to preview: https://sentry-docs-git-ivana-python-cron-upsert.sentry.dev/platforms/python/crons/

Extra resources

@vercel
Copy link

vercel bot commented Apr 2, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sentry-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 3, 2024 11:51am

@codecov
Copy link

codecov bot commented Apr 2, 2024

Bundle Report

Changes will decrease total bundle size by 6 bytes ⬇️

Bundle name Size Change
sentry-docs-server 6.11MB 5 bytes ⬆️
sentry-docs-edge-server 618.05kB 3 bytes ⬇️
sentry-docs-client 5.52MB 8 bytes ⬇️

@sentrivana sentrivana marked this pull request as ready for review April 2, 2024 11:22
@sentrivana sentrivana requested a review from antonpirker April 2, 2024 11:23
Copy link
Contributor

@antonpirker antonpirker left a comment

Choose a reason for hiding this comment

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

Not sure about a few things.

Comment on lines +15 to +37
## Check-Ins

Check-in monitoring allows you to track a job's progress by capturing two check-ins: one at the start of your job and another at the end of your job. This two-step process allows Sentry to notify you if your job didn't start when expected (missed) or if it exceeded its maximum runtime (failed).

If you use the `monitor` decorator/context manager, the SDK will create check-ins for the wrapped code automatically.

```python
from sentry_sdk.crons import capture_checkin
from sentry_sdk.crons.consts import MonitorStatus

check_in_id = capture_checkin(
monitor_slug='<monitor-slug>',
status=MonitorStatus.IN_PROGRESS,
)

# Execute your task here...

capture_checkin(
monitor_slug='<monitor-slug>',
check_in_id=check_in_id,
status=MonitorStatus.OK,
)
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure why this section is here. Maybe call it "Manual check-ins" and move it below the "Upserting Cron Monitors"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just copied the structure from https://docs.sentry.io/platforms/node/crons/ and https://docs.sentry.io/platforms/php/crons/ essentially. But agree this would be better below and renamed. Will do.

)
```

## Upserting Cron Monitors
Copy link
Contributor

Choose a reason for hiding this comment

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

I would avoid the word "upsert" and rather call this section: "Adding cron monitor configuration options" or something like this. (and later in the text mention that when you add the monitor_config and no monitor is existing it is created.)

Copy link
Contributor Author

@sentrivana sentrivana Apr 2, 2024

Choose a reason for hiding this comment

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

Changed to Configuring Cron Monitors and added a sentence: If the monitor doesn't exist in Sentry yet, it will be created.

Copy link
Contributor

@vivianyentran vivianyentran left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for adding!

@sentrivana
Copy link
Contributor Author

Not released yet -- will merge as soon as there is a release out there with upsert support.

@sentrivana sentrivana merged commit bf6738e into master Apr 10, 2024
@sentrivana sentrivana deleted the ivana/python-cron-upsert branch April 10, 2024 12:59
@github-actions github-actions bot locked and limited conversation to collaborators Apr 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants