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(python): Improve crons docs, add upserts #9597

Merged
merged 6 commits into from
Apr 10, 2024

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

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

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 ⬇️

Copy link
Member

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

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
Member

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
6 checks passed
@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.

None yet

3 participants