-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Fetch API refactor, WIP #4602
Fetch API refactor, WIP #4602
Conversation
Withing #4602 this method was changed from a class- to instance-method. The comment should be removed as the method is not a class-method anymore.
It would be ideal if either a class or instance could be passed, for backwards compatibility with 3rd party gems. |
Shouldn't this have been a major version change as it's not backwards compatible? I didn't expect a version change from 6.1.0 to 6.1.1 to break anything. A fix as suggested by @jhoffner to be released in version 6.1.2 would be great! |
The Fetch API has never been documented, it's not intended as a public, "user-facing" API. Internals can change. Furthermore I want to point out that Sidekiq is not semantic versioned. I do my best to limit any possible breaking change to minor or major releases based on the expected impact, patch releases try not to break anything. |
Thanks for the explanation, got it 👍 |
….0 that is not backwards compatible. As part of this refactoring the object in the :fetch option is expected to be an instance of Sidekiq::BasicFetch or an object that responds to the same methods. This change allows using the gem with newer versions while keeping compatibility with old ones. See sidekiq/sidekiq#4602 for more details.
….0 that is not backwards compatible. As part of this refactoring the object in the :fetch option is expected to be an instance of Sidekiq::BasicFetch or an object that responds to the same methods. This change allows using the gem with newer versions while keeping compatibility with old ones. See sidekiq/sidekiq#4602 for more details.
….0 that is not backwards compatible. As part of this refactoring the object in the :fetch option is expected to be an instance of Sidekiq::BasicFetch or an object that responds to the same methods. This change allows using the gem with newer versions while keeping compatibility with old ones. See sidekiq/sidekiq#4602 for more details.
The existing fetch API grew organically to its current state but is less than optimal: you pass a class reference via the global options, making it hard to configure a fetcher instance and requiring some test gymnastics to set up that global config.
I'd like to change the fetch API to be instance-based.
bulk_requeue
will become an instance method. Feedback welcome.