-
Notifications
You must be signed in to change notification settings - Fork 199
Add support for new Puma 7 hook names #635
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
base: main
Are you sure you want to change the base?
Conversation
I just wanted to note that those webhooks are deprecated, but will still work with a warning. They are not completely removed. |
@tannakartikey hm, that's weird, I'm currently getting this error using puma 7.0.0 and solid_queue 1.2.1:
Let me look again at what exactly is causing it 🤔 |
Same here,
My app don't use SolidQueue for the moment so I Comment the I also tried your code from this PR on my local app with some modification to run the solidQueue puma plugin and all seems good from my point of view. Thanks for your work and reactivity ;) |
@toydestroyer I think it is a breaking change after all! |
Same issue here, making deployments fail. |
The backward compatibility has been merged into Puma |
The fix in puma is released in 7.0.1 |
Thanks @tannakartikey and @schneems for the quick fix and release 🙏 I’ll leave it to @rosa to decide what to do with this PR. I’m happy to finalize it to remove deprecation notices. Or maybe Puma 6 support will be dropped in the next major Solid Queue version, so there’s no need for such code duplication? |
Thanks @tannakartikey and @schneems |
Thank you so much everyone 🙏 @toydestroyer, yes, I'm good with this PR to address the deprecation warnings. I'll drop support for Puma 6 in Solid Queue 2.0 when I finish all the stuff I want to include there. For now, it's fine to have the code to handle both.
I think just updating Puma and testing with version 7 is ok, since we'll drop support for < 7. |
That is done! |
Puma 7 is no longer compatible with Solid Queue due to the change in hook names (#633).
This PR is my attempt to fix it and support both new and old hook names.
Need advice on tests: should I just add Puma 7 to Appraisal, or should it be a matrix that tests all possible combinations of Puma and Rails versions?
Reference: https://github.com/puma/puma/releases/tag/v7.0.0