-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
build(deps): update redis client to v4 in legacy mode #21548
build(deps): update redis client to v4 in legacy mode #21548
Conversation
0e00144
to
cfa770d
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## develop #21548 +/- ##
========================================
Coverage 63.86% 63.86%
========================================
Files 765 765
Lines 69353 69353
Branches 6271 6271
========================================
Hits 44291 44291
Misses 21487 21487
Partials 3575 3575
Flags with carried forward coverage won't be shown. Click here to find out more. |
This comment was marked as outdated.
This comment was marked as outdated.
cfa770d
to
a33c45d
Compare
@blaggacao the fix in bench wont fix existing installs. What breaks with localhost? It seems to work fine for me. Can you share some error/traceback? |
@surajshetty3416 we might need the fix to make node work on systems which are preferring ipv6 (idk if there's any real cost to forcing ipv4 in code since we are using it only on localhost 馃し) |
Going forward, I'd recommend to not use dns (i.e. It's also a good way to avoid 'hard' problems. 馃槃 |
a33c45d
to
fa6c389
Compare
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing. |
@ankush Hopefully this is a more palatable version of #20925 馃槃 and the further refactoring to actually use the v4 api of that PR can then be added eventually as a second step (if needed).
The version bump also requires frappe/bench#1466 due to
localhost
being interpreted as ipv6::1
.I added a temp fix for CI that must be reverted once that bench PR is merged and has duly propagated.