-
Notifications
You must be signed in to change notification settings - Fork 58
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
CI: Add Support for Valkey 6.2, 7.0 and 7.2 #1562
Conversation
13a5ab0
to
9fbf1d7
Compare
target: | ||
description: "Specified target toolchain, ex. x86_64-unknown-linux-gnu" | ||
type: string | ||
required: true | ||
options: | ||
- x86_64-unknown-linux-gnu | ||
- aarch64-unknown-linux-gnu | ||
- x86_64-apple-darwin | ||
- aarch64-apple-darwin | ||
- aarch64-unknown-linux-musl | ||
- x86_64-unknown-linux-musl |
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 used as a cache key, so could be anything. This parameter and options looks like copied from rust toolchain selector.
I think you can remove this parameter and use combination of "os/runner + version" as a cache key
run: | | ||
cd ~/valkey | ||
sudo make install | ||
echo 'export PATH=/usr/local/bin:$PATH' >>~/.bash_profile |
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.
why do we need this? AFAIK make install
copies binaries to /usr/bin
, so they become available in $PATH
.github/workflows/java.yml
Outdated
uses: ./.github/workflows/install-redis | ||
with: | ||
redis-version: ${{ matrix.engine.version }} | ||
|
||
- name: Install Valkey | ||
if: ${{ matrix.engine.type == 'valkey' }} | ||
uses: ./.github/workflows/install-valkey |
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.
Maybe we need to combine these steps into install-engine
or install-db
?
@@ -11,6 +11,7 @@ on: | |||
- .github/workflows/node.yml | |||
- .github/workflows/build-node-wrapper/action.yml | |||
- .github/workflows/install-shared-dependencies/action.yml | |||
- .github/workflows/install-valkey/action.yml |
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.
Add to other CI workflows please (java, C#, go - all)
As for previews version of 7.2.5 valkey and redis are the same, and from there we want to test just against valkey ATM, so why not just change all to valkey and thats it? |
3f80f31
to
1c75943
Compare
Moved to #1711 |
CI: Add Support for Valkey 7.2.5
This PR introduces Valkey 7.2.5 to the CI matrix tests. Specifically, it adds Valkey to the matrix for the main code integration tests across all wrappers.
Included in this PR:
Not included in this PR:
These additional integrations can be addressed in a follow-up PR.