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

Don’t attempt to enforce concurrency limits with other queue adapters #333

Merged
merged 1 commit into from
Aug 18, 2021
Merged

Don’t attempt to enforce concurrency limits with other queue adapters #333

merged 1 commit into from
Aug 18, 2021

Conversation

codyrobbins
Copy link
Contributor

@codyrobbins codyrobbins commented Aug 18, 2021

One other minor issue I came across with the concurrency extension is that it is always being applied regardless of which queue adapter is actually handling the job. This became a problem when I got around to testing because in our particular test suite we’re using ActiveJob::QueueAdapters::TestAdapter. In this case jobs don’t end up being performed because there are no associated GoodJob::Job rows with advisory locks. This change skips enforcing the concurrency limits unless the job is being handled by the GoodJob adapter.

@codyrobbins
Copy link
Contributor Author

Just noticed this causes a bunch of test failures so I’ll have to look into getting those passing tomorrow!

Copy link
Owner

@bensheldon bensheldon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice discovery! This is a great improvement with a little cleanup.

lib/good_job/active_job_extensions/concurrency.rb Outdated Show resolved Hide resolved
lib/good_job/active_job_extensions/concurrency.rb Outdated Show resolved Hide resolved
@bensheldon bensheldon added the bug Something isn't working label Aug 18, 2021
@bensheldon bensheldon merged commit 8e1a0f3 into bensheldon:main Aug 18, 2021
@bensheldon
Copy link
Owner

Released in v1.13.1

Thank you! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants