-
-
Notifications
You must be signed in to change notification settings - Fork 226
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(api): Support monitor upserts #1807
Conversation
None of the cron parsing crates I could find seemed very good, so I wrote my own cron parser and opened a draft PR for it here: getsentry/sentry-rust#625. I probably need to do some polishing up on that PR, but once we are able to merge it, we can use that code to validate the crontab schedule provided by the user |
I agree with the decision to do this ourselves, though I'm normally more inclined to just grab something off the rack, as it were. But all the crates for this crontab stuff are ancient. |
100% agree with your point; I would have also preferred to use a crate if a suitable one existed. Maybe we can consider publishing the cron validator I wrote as its own crate so others can use it |
Co-authored-by: Sebastian Zivota <[email protected]>
Cargo.toml
Outdated
# TODO: Revert back to crates.io version prior to merge | ||
sentry = {git = "https://github.com/getsentry/sentry-rust.git", rev = "refs/pull/625/head", default-features = false, features = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have pinned here my pull request on sentry-rust
, which adds the crontab validation, so that we can test this PR before the sentry-rust
changes are merged.
We need to merge the sentry-rust
PR before merging this PR and release a new sentry-rust
version, and we should pin the new version here before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good apart from my one comment :)
Co-authored-by: Sebastian Zivota <[email protected]>
@szokeasaurusrex would you also be able to update the CLI documentation to include details about how to use this |
@evanpurkhiser I opened an issue so I remember to add these docs; I should get around to adding it later this week. |
Perfect. Thank you! |
The Sentry CLI can now support creating cron monitors while executing the
monitors run
command via the new--schedule
or the equivalent abbreviated-s
argument. The argument expects to receive the schedule in crontab syntax, which the CLI validates. This PR also introduces the--check-in-margin
,--max-runtime
, and--timezone
arguments to allow for additional configuration of the cron monitor.Closes #1801