search

getsentry / sentry-ruby

Sentry SDK for Ruby
https://sentry.io/for/ruby
MIT License
926 stars 493 forks source link

(fix) crons: require sidekiq-cron early #2173

Closed natikgadzhi closed 10 months ago

natikgadzhi commented 10 months ago

skip-changelog

(am I doing this right?)

Summary

Addressing this review by @st0012 on sidekiq-cron integration — the corresponsing fix for sidekiq-scheduler is in #2172.

codecov[bot] commented 10 months ago

Codecov Report

Merging #2173 (4bd493a) into master (c7c8e5b) will decrease coverage by 0.06%. Report is 1 commits behind head on master. The diff coverage is 50.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2173 +/- ## ========================================== - Coverage 97.40% 97.34% -0.06% ========================================== Files 98 98 Lines 3655 3656 +1 ========================================== - Hits 3560 3559 -1 - Misses 95 97 +2 ``` | [Components](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2173/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | Coverage Δ | | |---|---|---| | [sentry-ruby](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2173/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | `98.02% <ø> (ø)` | | | [sentry-rails](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2173/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | `94.98% <ø> (ø)` | | | [sentry-sidekiq](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2173/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | `94.93% <50.00%> (-0.61%)` | :arrow_down: | | [sentry-resque](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2173/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | `92.06% <ø> (-1.59%)` | :arrow_down: | | [sentry-delayed_job](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2173/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | `94.36% <ø> (ø)` | | | [sentry-opentelemetry](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2173/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | `100.00% <ø> (ø)` | | | [Files](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2173?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | Coverage Δ | | |---|---|---| | [sentry-sidekiq/lib/sentry/sidekiq/cron/job.rb](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2173?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry#diff-c2VudHJ5LXNpZGVraXEvbGliL3NlbnRyeS9zaWRla2lxL2Nyb24vam9iLnJi) | `92.85% <50.00%> (-7.15%)` | :arrow_down: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2173/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry)
st0012 commented 10 months ago

In the case of Ruby 2.6, it'll install an older version of sidekiq-cron (1.9.1), which will not be compatible with our current tests due to the change in load_from_hash!. I think we should make job_spec compatible with old sidekiq-cron too. Could you also update it for us too? If not, I'll open a separate PR for it later.

natikgadzhi commented 10 months ago

Yep, I’m on it.

natikgadzhi commented 10 months ago

@st0012 should be good to go now.