Remove config get/set depication warning due to Sidekiq 6.5+

[t27duck]

Closes #384

Sidekiq 6.5 and later changed how config options work.

 # Before
config.options[:x] = 1
config.options[:y] # => 2

 # After
config[:x] = 1
config[:y] # => 2

The existing functionality works, but now throws a deprication warning and will most likely be removed for Sidekiq 7. There is also no non-depricated way to access the config hash.

To maintain compatability with older Sidekiq versions, a SIDEKIQ_GTE_6_5_0 has been added which is true if Sidekiq::VERSION is 6.5 or later. This constant is then used to determine now to pull the Sidekiq config hash and set configuration options where applicable.

This should allow the gem to maintain compatability with Sidekiq < 6.5

[t27duck]

I have a very basic Rails app with sidekiq 6.5 and this branch wired up to enqueue an ActiveJob and stright Sidekiq job every 5 minutes.

https://github.com/t27duck/sidekiq_scheduler_384_test

• Pull
• Run bundle install
• bundle exec sidekiq
• rails s and http://localhost:3000/sidekiq should work including the schedules page

The jobs enqueue and run as expected. Deprication warnings no longer showed up.

🤷‍♂️

[t27duck]

What I find odd is the specs are using the now depricated setters/getters for the Sidekiq config but I didn't see any deprication warnings printed out?

[tagliala]

What I find odd is the specs are using the now depricated setters/getters for the Sidekiq config but I didn't see any deprication warnings printed out?

I've tried to remove the code inside config.on(:startup) block and the specs keep passing

[marcelolx]

@t27duck Thanks for the PR! I probably won't have time this week to look into it, but as soon as I have, I'll give you feedback!

[olleolleolle]

Looks good! 👍

[marcelolx]

What I find odd is the specs are using the now depricated setters/getters for the Sidekiq config but I didn't see any deprication warnings printed out?

That's due to the log level in specs be in ERROR mode https://github.com/moove-it/sidekiq-scheduler/blob/master/spec/support/initialization/logs.rb#L5

[bpo]

I'm concerned that instance_variable_get(:@config) works around the deprecation warning without addressing the refactor.

Shouldn't 6.5+ initialize SidekiqScheduler::Manager with a config object rather than an options hash?

[t27duck]

SidekiqScheduler::Manager is assuming a hash-like object that can be merged into another hash.

Specifically, the whole thing breaks down here https://github.com/moove-it/sidekiq-scheduler/blob/master/lib/sidekiq-scheduler/manager.rb#L40

Sidekiq 6.5+'s new config object only exposes hash-like setters and getters for its keys, but there's no way to get the entire key-value set of options.

[marcelolx]

@bpo @t27duck I think that for now to just fix the warning using instance_variable_get(:@config) is the best option we have ;/

We definitely want to pass the config object but that means a large refactoring to the manager which probably will be incompatible with older versions of sidekiq and will require a major bump.

[marcelolx]

I want to make some more tests tomorrow, if everything looks good I'll push a new release, thanks @t27duck!