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

fix(after_unlock): regression from #707 #737

Merged

Conversation

adamcreekroad
Copy link
Contributor

Background

#707 introduced a regression where the instance level after_unlock callback is no longer being called. This was due to the change of using item[CLASS] instead of worker_class to set job_class in SidekiqUniqueJobs::Middelware#call. item[CLASS] is a string, where as worker_class can be either an instance, or the class itself. For the instance level after_unlock to work, we need job_class to be an instance of the worker.

This fixes the issue by switching back to setting job_class to worker_class

Changelog

  • Use worker_class for self.job_class in Middleware#call

adamcreekroad and others added 2 commits October 26, 2022 18:39
BACKGROUND -------------------------------------------------------------

mhenrixon#707 introduced a regression where the instance level `after_unlock`
callback is no longer being called. This was due to the change of using
`item[CLASS]` instead of `worker_class` to set `job_class` in
`SidekiqUniqueJobs::Middelware#call`. `item[CLASS]` is a string, where
as `worker_class` can be either an instance, or the class itself. For the
instance level `after_unlock` to work, we need `job_class` to be an
instance of the worker.

This fixes the issue by switching back to setting `job_class` to
`worker_class`

CHANGELOG --------------------------------------------------------------

- Use worker_class for self.job_class in Middleware#call
@mhenrixon
Copy link
Owner

mhenrixon commented Dec 3, 2022

Hi @adamcreekroad,

I updated your branch with the latest changes to make sure the CI results are fair to your change.

Please fix any issues remaining, and I'll merge and release your PR as well. Thank you for your contribution.

@mhenrixon mhenrixon enabled auto-merge (squash) December 3, 2022 09:50
@mhenrixon mhenrixon changed the title Fix instance level after_unlock callback fix(:after_unlock): fix regression from #707 Dec 3, 2022
@mhenrixon mhenrixon changed the title fix(:after_unlock): fix regression from #707 fix(after_unlock): fix regression from #707 Dec 3, 2022
@mhenrixon mhenrixon merged commit 6f244a2 into mhenrixon:main Dec 3, 2022
@mhenrixon mhenrixon changed the title fix(after_unlock): fix regression from #707 fix(after_unlock): regression from #707 Dec 3, 2022
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.

2 participants