-
-
Notifications
You must be signed in to change notification settings - Fork 719
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 redis
and hiredis-client
version requirements
#10455
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.
Same problem on my machine too.
rake aborted!
LoadError: cannot load such file -- redis/connection/hiredis
As per the hiredis-rb docs, we have configured our redis gem to load via hiredis:
https://github.com/openfoodfoundation/openfoodnetwork/blob/dependabot/bundler/redis-5.0.6/Gemfile#L99
gem 'redis', '>= 4.0', require: ['redis', 'redis/connection/hiredis']
But for some reason, it can no longer find it there. Strange, because we haven't updated hiredis-rb at all.
In fact, they haven't released an update for some years now. It looks like there are recent changes, so I tried installing from the master branch:
gem 'hiredis', github: "/redis/hiredis-rb", branch: "master"
But it Failed to build gem native extension.
:(
I don't know what to do next. Anybody else?
Seems like upgrading to So, I've updated the PR to upgrade redis until |
redis
and hiredis-client
version requirements
Great work team 💪 @mkllnk , can you think of anything else that should be manually tested for a Redis upgrade? I can't. PS I just noticed while reloading that the bundler version has been bumped, which may not be intentional. It's such a minor update that I think it's ok to include here though. |
Unfortunately deployment failed ❌
|
Maybe for redis < 5, we still need |
Ok, so we can't upgrade to Redis 5 until we upgrade to Rails 7.0.4. So I've marked this as blocked by that issue and added a label. We are already on the latest for v4.x. We could update the gemfile and let Dependabot alert us about new v4.x versions. But because it sounds like we're close on Rails 7, I think we can just leave this for now. |
I agree! thanks 👍 |
@dependabot rebase |
Looks like this PR has been edited by someone other than Dependabot. That means Dependabot can't rebase it - sorry! If you're happy for Dependabot to recreate it from scratch, overwriting any edits, you can request |
since Redis >= 3, < 5 is actually a requirement of Action Cable Redis subscription adapter until Rails 7.0.4. https://github.com/rails/rails/blob/v6.1.7/actioncable/lib/action_cable/subscription_adapter/redis.rb
7713de5
to
4564021
Compare
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.
Yay!
Just noting that this is also upgrading our version of Bundler from 2.4.3
to 2.4.5
. Which I think is fine.
https://github.com/rubygems/rubygems/blob/master/bundler/CHANGELOG.md#245-january-21-2023
@@ -927,4 +929,4 @@ RUBY VERSION | |||
ruby 3.0.3p157 | |||
|
|||
BUNDLED WITH | |||
2.4.3 | |||
2.4.5 |
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.
As far as I can see Dependabot doesn't support managing this yet. I'm curious why you have a later version actually?
I'm not sure if we have any particular policy, but I guess this normally only gets updated as a result of updating Ruby.
Maybe we should update more frequently. I can see there's one vulnerability mentioned on the changelog, which is fixed in 2.4.11
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, I think that this should be deployed on all staging servers to check that it works.
Gemfile
Outdated
@@ -97,7 +97,7 @@ gem 'rack-timeout' | |||
gem 'roadie-rails' | |||
|
|||
gem 'puma' | |||
gem 'redis', "< 5" | |||
gem 'redis', "< 5" # a requirement of Action Cable Redis subscription adapter until Rails 7.0.4. |
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.
Hm, we are on Rails 7.0.5 already...
I'm wondering what to test. Maybe let's test bulk invoice printing because it uses cable_ready. |
Or vouchers. |
Thank you team, and thanks @mkllnk for providing some test instructions. I have tested both, bulk invoice printing and vouchers (backoffice and checkout) and everything seems to work normal. Merging! 🚀 |
Just noting that there was a Bugnag error when this branch was deployed: |
Thanks @Matt-Yorkley , |
It seems this PR introduced flakyness affecting several specs on the build, opened an issue here. |
That error is still there. Just happened again. It seems to happen every few hours and then four times in a row. I wonder if we should revert until we know what's going on. I'm drafting a release today, so I'll look into it for a bit but if I can't see an immediate solution I may revert this. |
I'll revert this. we have several errors which seem to relate to redis and caching: |
Bumps redis from 4.8.1 to 5.0.6.
Changelog
Sourced from redis's changelog.
Commits
6dbbc94
Release 5.0.69c49947
Merge pull request #1175 from casperisfine/blocking-call-timeoutd3efeeb
Add wait on blocking commands for an extraconfig.read_timeout
f78beb1
Merge pull request #1176 from casperisfine/ruby-3.2daf7e02
Use Ruby 2.6 to run rubocop031911a
Add Ruby 3.2 to CI5a9fb00
Merge pull request #1170 from wcmonty/wm/add_minid_option_to_xtrime76e1cb
Add MINID and LIMIT options to xtrim3c42db6
Treat ReadOnlyError as a ConnectionErrora8e00cb
Merge pull request #1167 from wmontgomery-splunk/wm/add_nomkstream_to_xaddYou can trigger a rebase of this PR by commenting
@dependabot rebase
.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebase
will rebase this PR@dependabot recreate
will recreate this PR, overwriting any edits that have been made to it@dependabot merge
will merge this PR after your CI passes on it@dependabot squash and merge
will squash and merge this PR after your CI passes on it@dependabot cancel merge
will cancel a previously requested merge and block automerging@dependabot reopen
will reopen this PR if it is closed@dependabot close
will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot ignore this major version
will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor version
will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependency
will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)