-
-
Notifications
You must be signed in to change notification settings - Fork 378
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
Upgrade Dependency connect-redis from 3.4.0 to 6.1.3 #1387
Upgrade Dependency connect-redis from 3.4.0 to 6.1.3 #1387
Conversation
633d761
to
43adde8
Compare
17cf0dc
to
ec019cd
Compare
The purpose is to remove a vulnerable version of redis from dependencies of connect-redis. the choosen version is the one updating node-redis to a safe version
ec019cd
to
72d3d3c
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.
LGTM (with a bit of nitpicking)
Importing this way, it's the declaration.d.ts that is used. Thats why I added unref in it.
6.1.3 is choosen cause it's the last version supporting redis@v3
While the redis version was not fixed, produced Docker images onboard 3.1.0 instead of >= 3.1.1.
@@ -197,7 +197,7 @@ | |||
"prom-client": "14.2.0", | |||
"qrcode": "1.5.0", | |||
"randomcolor": "0.5.3", | |||
"redis": "3.1.1", | |||
"redis": "~3.1.2", |
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.
While the redis version was not prefixed with ~, produced Docker images contained
3.1.0 instead of >= 3.1.1.
@@ -142,7 +142,7 @@ | |||
"color-convert": "2.0.1", | |||
"commander": "9.3.0", | |||
"components-jqueryui": "1.12.1", | |||
"connect-redis": "3.4.0", | |||
"connect-redis": "6.1.3", |
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.
This is the last version of connect-redis
supporting redis@v3
The CI error is a known flaky end-to-end test (importer2). |
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.
LGTM
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.
Confirming tests still pass in our internal CI after these changes.
Thanks @hexaltation.
Context
This PR is a follow-up of #1368
It manage to upgrade redis-connect to it's latest release
Proposed solution
The following strategy has been chosen.
Bump connect-redis from 3.4.0 to 6.1.3
Applies migration following https://github.com/tj/connect-redis/blob/c951850eb72759f387d4ae0c249aca8e1e9fc244/migration-to-v4.md
Has this been tested?