Support crons with timezone for sidekiq-scheduler #2209
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #2209 +/- ##
=======================================
Coverage 97.39% 97.39%
=======================================
Files 101 101
Lines 3799 3805 +6
=======================================
+ Hits 3700 3706 +6
Misses 99 99
|
Co-authored-by: Stefan Jandl <[email protected]>
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
- Support crons with timezone for sidekiq-scheduler ([#2209](https://github.com/getsentry/sentry-ruby/pull/2209))If none of the above apply, you can opt out of this check by adding |
| @@ -133,8 +133,10 @@ def perform | |||
| end | |||
| end | |||
|
|
|||
| class HappyWorkerForCron < HappyWorker; end | |||
There was a problem hiding this comment.
Why do we need another HappyWorkerForCron?
There was a problem hiding this comment.
to keep each class isolated after patching, since we don't have a 'reverse prepend'
There was a problem hiding this comment.
No I mean we already had this worker before this PR, so now we have 2:
sentry-ruby/sentry-sidekiq/spec/spec_helper.rb
Lines 136 to 137 in 9b24de9
There was a problem hiding this comment.
ah hmm that was probably copypasta error, thx
| @@ -37,7 +37,18 @@ def new_job(name, interval_type, config, schedule, options) | |||
| # so we convert it to minutes before passing in to the monitor. | |||
| monitor_config = case interval_type | |||
| when "cron" | |||
| Sentry::Cron::MonitorConfig.from_crontab(schedule) | |||
| parsed_cron = ::Fugit.parse_cron(schedule) | |||
There was a problem hiding this comment.
Can we mention the origin of Fugit in a comment? It's a second-level dependency of sidekiq-scheduler, through the rufus-scheduler gem.
closes #2187