-
Notifications
You must be signed in to change notification settings - Fork 337
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
Support rack 3 #586
Support rack 3 #586
Conversation
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.
Cool, thank you!
I think to have the test pass we would additionally need to point the rack_3 appraisal to a version of actionpack that actually compatible with rack 3.
This depends on rails/rails#45741 being merged first probably. |
What's blocking this from being merged? |
I think CI needs to be green for the PR before we can consider merging. No issues with the PR itself, though. |
5d3da49
to
39901cd
Compare
It looks like this is blocked on rails/rails#46594 and indirectly rails/rails#46206 - @rafaelfranca FYI. I'm going to try and rework the dependencies so we don't pull in railties when testing rack 3... it might allow us to get tests passing. |
From what I'm seeing, this will work if rails |
Any updates here? Similarly blocked trying to remediate CVE-2022-30122 by upgrading to rack 3.0. |
Since rails/rails#46206 was merged, it's just rails/rails#46594 blocking this one? |
Sorry, I totally forgot about this PR. I'll take a look at it in the next few days. |
@ioquatix Hello, any updates on this? |
Same, any update? And thanks for the library! |
Also checking in, would love to get our apps up to Rack 3, blocked on this |
We should probably also update CI, but we've been testing
rack-attack
from therack
repo (external tests) and all the tests are passing. So this should probably be good.Closes #596