Skip to content
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

(core mod) pkg: bump go-redis -> v9 #627

Merged
merged 8 commits into from
Apr 18, 2023

Conversation

kamikazechaser
Copy link
Collaborator

@kamikazechaser kamikazechaser commented Mar 14, 2023

This is a replacement PR for #602 and closes #570. It fixes the tests (all tests pass) and removes v8 references form wherever possible.

Modules tools and x still indirectly reference v8. They should be updated once the main asynq package is bumped. I have also removed replace directives that were initially added to this PR ( only useful for local dev) and which were previously also removed in #392.

l0nax and others added 2 commits January 31, 2023 11:20
Version v9 implements the support for Redis v7 and has some
other improvements.
@kamikazechaser kamikazechaser deleted the sohail/bump-redis-pkg branch March 14, 2023 14:55
@kamikazechaser kamikazechaser restored the sohail/bump-redis-pkg branch March 14, 2023 14:57
trungdlp-wolffun and others added 3 commits March 31, 2023 07:15
Signed-off-by: Mohammed Sohail <sohailsameja@gmail.com>
Signed-off-by: Mohammed Sohail <sohailsameja@gmail.com>
@kamikazechaser kamikazechaser changed the title bump redis pkg pkg: bump go-redis -> v9 Mar 31, 2023
@kamikazechaser kamikazechaser marked this pull request as draft March 31, 2023 08:02
* this was previously reverted also in hibiken#392
@kamikazechaser kamikazechaser marked this pull request as ready for review March 31, 2023 10:10
@ioannidesalex
Copy link

Bump. This is really good to have.

@trungdlp-wolffun
Copy link
Contributor

LGTM @kamikazechaser

@hibiken
Copy link
Owner

hibiken commented Apr 7, 2023

Thanks for creating this PR.
Can you investigate why the build is failing? I'll merge after the build passes

@kamikazechaser
Copy link
Collaborator Author

For build One of the reasons was related to redis/go-redis#2458. I have pushed the fix. Doesn't break on my local which is 1.20.1.

I'll investigate the build-tools one too.

@kamikazechaser
Copy link
Collaborator Author

@hibiken Btw since we are not dealing with any secrets in the CI workflow, could we open up permissions to run the workflows without explicit approvals, wdyt?

@hibiken
Copy link
Owner

hibiken commented Apr 8, 2023

@kamikazechaser thanks for looking into this.
Looks like the first contributor will always require approvals to run actions (regardless of which option I pick here). Sorry for the inconvenience!

Screen Shot 2023-04-08 at 3 59 46 AM

* revert state to as it was before v9 updates for tools and x modules
@kamikazechaser
Copy link
Collaborator Author

Ah ok.

I see the core build is now passing but the x and tools is not/will not because of how it references the asynq package version. I think because of how x and tools depend on the core module. we need to merge this in (after tests pass), then individually update tools and x. So for now I have reverted the go-redis updates made to x and tools.

Finally, we could tag this as v0.26.0 then update x and tools to ref that, then another update to v0.27.0. We then retract v0.26.0 just to be on the safe side with potential side effects of different go-redis across the modules.

@kamikazechaser kamikazechaser changed the title pkg: bump go-redis -> v9 (core mod) pkg: bump go-redis -> v9 Apr 10, 2023
@kamikazechaser
Copy link
Collaborator Author

@hibiken Waiting for CI run approval 😅.

@codecov
Copy link

codecov bot commented Apr 18, 2023

Codecov Report

Merging #627 (56cd6a1) into master (cc777eb) will not change coverage.
The diff coverage is 50.00%.

@@           Coverage Diff           @@
##           master     #627   +/-   ##
=======================================
  Coverage   68.34%   68.34%           
=======================================
  Files          27       27           
  Lines        3829     3829           
=======================================
  Hits         2617     2617           
  Misses        928      928           
  Partials      284      284           
Impacted Files Coverage Δ
asynq.go 68.78% <ø> (ø)
client.go 89.47% <ø> (ø)
inspector.go 60.69% <ø> (ø)
internal/base/base.go 70.18% <ø> (ø)
internal/rdb/inspect.go 68.69% <ø> (ø)
internal/testutil/testutil.go 0.00% <0.00%> (ø)
scheduler.go 82.85% <ø> (ø)
server.go 85.47% <ø> (ø)
subscriber.go 100.00% <ø> (ø)
internal/rdb/rdb.go 74.27% <60.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@hibiken hibiken merged commit cbb1be3 into hibiken:master Apr 18, 2023
@trungdlp-wolffun
Copy link
Contributor

Ah ok.

I see the core build is now passing but the x and tools is not/will not because of how it references the asynq package version. I think because of how x and tools depend on the core module. we need to merge this in (after tests pass), then individually update tools and x. So for now I have reverted the go-redis updates made to x and tools.

Finally, we could tag this as v0.26.0 then update x and tools to ref that, then another update to v0.27.0. We then retract v0.26.0 just to be on the safe side with potential side effects of different go-redis across the modules.

It time's for update 🥳

@kamikazechaser
Copy link
Collaborator Author

@trungdlp-wolffun Might need to wait for a tagged release first on the core module.

@trungdlp-wolffun
Copy link
Contributor

v0.24.1 was released @kamikazechaser 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE REQUEST] Update Redis GO library to support Redis 7
5 participants