Skip to content
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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DmitryTsepelev
Copy link

Related to the Zeitwerk issue about occasional NameError inside Sidekiq

In 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.

@@ -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
Copy link
Author

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?
Copy link
Author

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

elcuervo added a commit to unsplash/sidekiq-grouping that referenced this pull request Jul 10, 2023
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
@MrPhantomT
Copy link

Hi there are there any plans to merge this pull-request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants