-
Notifications
You must be signed in to change notification settings - Fork 53
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
Delay initialization until Rails is fully initialized #41
base: master
Are you sure you want to change the base?
Delay initialization until Rails is fully initialized #41
Conversation
@@ -14,6 +14,26 @@ Useful for: | |||
*NOTE:* As of 1.0.6 works with Sidekiq 4. | |||
*NOTE:* As of 1.0.8 Locking is atomic (set nx/ex) and will no longer lead to batches that are permalocked and stuck | |||
|
|||
## Installation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved this section to the top because non-Rails users will have to perform an extra setup
@@ -50,4 +51,3 @@ def start! | |||
end | |||
end | |||
|
|||
Sidekiq::Grouping.start! if Sidekiq.server? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a breaking change, so I guess we'll need a major release
e3a1fbd
to
b0a91ee
Compare
Directly taken from gzigzigzeo#41 There's a potential issue with the initialization that prevents the config to be correctly loaded when in Rails. This enforces the right timing
Hi there are there any plans to merge this pull-request? |
Related to the Zeitwerk issue about occasional
NameError
inside SidekiqIn some cases sidekiq-grouping starts before Rails is fully initialized and Zeitwerk initial loading is completed, which can cause situations when some classes are loaded inside the concurrent thread rather than in Zeitwerk. This PR moves gem initialization to the Railtie.