Support Sidekiq Pro super_fetch

[mnovelo]

Distilling the discussions in this issue and the work in this fork, this adds support for Sidekiq Pro's super_fetch for v1 of sidekiq-throttled and v7.x of Sidekiq Pro.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (bf88735) 98.94% compared to head (3c649ed) 98.95%.

❗ Current head 3c649ed differs from pull request most recent head ea96cc3. Consider uploading reports for the commit ea96cc3 to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #178   +/-   ##
=======================================
  Coverage   98.94%   98.95%           
=======================================
  Files          18       18           
  Lines         380      381    +1     
  Branches       53       53           
=======================================
+ Hits          376      377    +1     
  Misses          4        4           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ixti]

After taking another look, I think there's a better way to exclude Sidekiq-Pro specs when it's not available. Let's add this change to spec_helper.rb:

diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index cb34b71..661327d 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -61,6 +61,9 @@ RSpec.configure do |config|
   # metadata: `fit`, `fdescribe` and `fcontext`, respectively.
   config.filter_run_when_matching :focus
 
+  # Skip tests that require Sidekiq-Pro
+  config.filter_run_excluding sidekiq_pro: true unless defined? Sidekiq::Pro
+
   # Allows RSpec to persist some state between runs in order to support
   # the `--only-failures` and `--next-failure` CLI options. We recommend
   # you configure your source control system to ignore this file.

Then spec/lib/sidekiq/throttled/patches/super_fetch_spec.rb will become:

RSpec.describe Sidekiq::Throttled::Patches::SuperFetch, :sidekiq_pro do
  let(:base_queue) { "default" }
  ...

So that spec suite will run only when Sidekiq-Pro is available, or when one runs it explicitly with:

bundle exec rspec --tag sidekiq_pro

[ixti]

Looks good to me. @freemanoid please take another look.

[ixti]

@mnovelo I've added the above proposals (and a bit more) to a separate branch

[ixti]

Can you please check if the updated instructions make sense and specs pass for you https://github.com/ixti/sidekiq-throttled/tree/sidekiq-pro-super-fetch - and if so - I will merge it

[mnovelo]

Thanks! I was just wrapping them up and taking care of the Rubocop Metrics/CyclomaticComplexity: Cyclomatic complexity for active_queues is too high. warning. Will push in a moment

[ixti]

Thanks! I was just wrapping them up and taking care of the Rubocop Metrics/CyclomaticComplexity: Cyclomatic complexity for active_queues is too high. warning. Will push in a moment

Oh. hm... I wonder why Metrics/CyclomaticComplexity fails. It's not that complex LOL

[mnovelo]

Just added a change that removes some redundant safe operators since nil.to_a.to_h becomes {}. That took care of it 😊

[ixti]

I'm gonna squash merge this PR in a bit!

[mnovelo]

sorry, one moment. Specs aren't passing now

[ixti]

Oh.. Yeah. I'll wait.

[ixti]

sorry, one moment. Specs aren't passing now

Make sure to cp .rspec.sidekiq-pro .rspec-local, by default :sidekiq_pro tests are disabled, and .rspec.sidekiq-pro is a version of .rspec that does not filter them out.

[mnovelo]

Oh it's the 6.5 specs that are failing. I had it set to skip Sidekiq Pro < 7 since it uses a different config. For some reason, now they're running

[mnovelo]

Weird fluke in my local environment. Now everything is back and passing 😖 Anyway, ready to merge!

[mnovelo]

@ixti I applied this suggestion using the GitHub UI but it didn't get signed. Do you need me to fix that or does it not matter since you're gonna squash and merge?

[ixti]

@ixti I applied this suggestion using the GitHub UI but it didn't get signed. Do you need me to fix that or does it not matter since you're gonna squash and merge?

@mnovelo Yeah. Gonna squash merge it. So. don't worry about commits not being signed :D

[ixti]

@mnovelo Thank you very much! I'm gonna cut 1.3.0 release. and in 2.0.0 sidekiq 6.x will be fully dropped, so I am not worried about it :D

[ixti]

Released. Thank you very much to all who participated in this journey! ❤️