-
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
Add Valkey8 support #2102
Add Valkey8 support #2102
Conversation
0f57133
to
9660c60
Compare
30943b8
to
1713afa
Compare
b834291
to
453160a
Compare
7552f1d
to
9e56a07
Compare
Signed-off-by: Shoham Elias <[email protected]>
@@ -62,6 +62,7 @@ runs: | |||
echo 'export PATH=/usr/local/bin:$PATH' >>~/.bash_profile | |||
|
|||
- name: Verify Valkey installation and symlinks | |||
if: ${{ !contains(inputs.engine-version, '-rc1') }} |
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.
it's too specific. change to
if: ${{ !contains(inputs.engine-version, '-rc1') }} | |
if: ${{ !contains(inputs.engine-version, '-rc') }} |
.toLowerCase() | ||
.contains("can't write against a read only replica")); | ||
if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) { | ||
assertEquals(OK, clusterClient.flushall(route).get()); |
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.
since valkey 8.0.8 flushall can run on replicas
if not await check_if_server_version_lt(glide_client, "7.9.0"): | ||
cluster_mode = parse_info_response(info_res)["server_mode"] | ||
else: | ||
cluster_mode = parse_info_response(info_res)["redis_mode"] | ||
expected_cluster_mode = isinstance(glide_client, GlideClusterClient) |
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.
we can remove it
assert "WATCH inside MULTI is not allowed" in str( | ||
e | ||
) # TODO : add an assert on EXEC ABORT | ||
|
||
else: | ||
assert "Command not allowed inside a transaction" in str( |
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.
we can leave with "not allowed", remove the whole string message
@@ -79,6 +79,9 @@ def get_random_string(length): | |||
async def check_if_server_version_lt(client: TGlideClient, min_version: str) -> bool: | |||
# TODO: change it to pytest fixture after we'll implement a sync client | |||
info_str = await client.info([InfoSection.SERVER]) | |||
valkey_version = parse_info_response(info_str).get("valkey_version") |
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.
valkey_version = parse_info_response(info_str).get("valkey_version")
server_version = valkey_version if valkey_version is not None else parse_info_response(info_str).get("redis_version")
if (error) { | ||
reject(error); | ||
} else { | ||
resolve(stdout.split("v=")[1].split(" ")[0]); |
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.
remove duplication
return cmd_args | ||
|
||
# Try starting valkey-server first | ||
server_name = "valkey-server" |
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.
put it in a dedicate function to check which server to use
[x] Added new command CLUSTER SLOT-STATS in each of the wrappers for CLUSTER mode only. PR #351implies a single slot.