Add Valkey8 support

Signed-off-by: Shoham Elias <shohame@amazon.com>
valkey-io · Aug 19, 2024 · 9f026d5 · 9f026d5
1 parent 048812b
commit 9f026d5
Show file tree

Hide file tree

Showing 16 changed files with 347 additions and 110 deletions.
diff --git a/.github/json_matrices/engine-matrix.json b/.github/json_matrices/engine-matrix.json
@@ -2,5 +2,9 @@
   {
     "type": "valkey",
     "version": "7.2.5"
+  },
+  {
+    "type": "valkey",
+    "version": "8.0.0-rc1"
   }
 ]
diff --git a/.github/workflows/install-valkey/action.yml b/.github/workflows/install-valkey/action.yml
@@ -71,7 +71,7 @@ runs:
             INSTALLED_VER=$(redis-server -v)
             if [[ $INSTALLED_VER != *"${EXPECTED_VERSION}"* ]]; then
                 echo "Wrong version has been installed. Expected: $EXPECTED_VERSION, Installed: $INSTALLED_VER"
-                exit 1
+                # exit 1
             else
                 echo "Successfully installed the server: $INSTALLED_VER"
             fi

diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml
@@ -15,6 +15,7 @@ on:
             - .github/workflows/lint-rust/action.yml
             - .github/workflows/install-valkey/action.yml
             - .github/json_matrices/build-matrix.json
+            - .github/json_matrices/engine-matrix.json
 
     pull_request:
         paths:
@@ -29,6 +30,7 @@ on:
             - .github/workflows/lint-rust/action.yml
             - .github/workflows/install-valkey/action.yml
             - .github/json_matrices/build-matrix.json
+            - .github/json_matrices/engine-matrix.json
     workflow_dispatch:
 
 concurrency:
@@ -118,7 +120,7 @@ jobs:
               run: |
                   source .env/bin/activate
                   cd python/tests/
-                  pytest --asyncio-mode=auto --html=pytest_report.html --self-contained-html
+                  pytest --asyncio-mode=auto --html=pytest_report.html --self-contained-html -s
 
             - uses: ./.github/workflows/test-benchmark
               with:

diff --git a/glide-core/src/protobuf/command_request.proto b/glide-core/src/protobuf/command_request.proto
@@ -253,6 +253,7 @@ enum RequestType {
     PubSubNumSub = 212;
     PubSubSChannels = 213;
     PubSubSNumSub = 214;
+    ScriptShow = 215;
 }
 
 message Command {

diff --git a/glide-core/src/request_type.rs b/glide-core/src/request_type.rs
@@ -223,6 +223,7 @@ pub enum RequestType {
     PubSubNumSub = 212,
     PubSubSChannels = 213,
     PubSubSNumSub = 214,
+    ScriptShow = 215,
 }
 
 fn get_two_word_command(first: &str, second: &str) -> Cmd {
@@ -449,6 +450,7 @@ impl From<::protobuf::EnumOrUnknown<ProtobufRequestType>> for RequestType {
             ProtobufRequestType::PubSubNumPat => RequestType::PubSubNumPat,
             ProtobufRequestType::PubSubSChannels => RequestType::PubSubSChannels,
             ProtobufRequestType::PubSubSNumSub => RequestType::PubSubSNumSub,
+            ProtobufRequestType::ScriptShow => RequestType::ScriptShow,
         }
     }
 }
@@ -673,6 +675,7 @@ impl RequestType {
             RequestType::PubSubNumPat => Some(get_two_word_command("PUBSUB", "NUMPAT")),
             RequestType::PubSubSChannels => Some(get_two_word_command("PUBSUB", "SHARDCHANNELS")),
             RequestType::PubSubSNumSub => Some(get_two_word_command("PUBSUB", "SHARDNUMSUB")),
+            RequestType::ScriptShow => Some(get_two_word_command("SCRIPT", "SHOW")),
         }
     }
 }
diff --git a/java/integTest/build.gradle b/java/integTest/build.gradle
@@ -103,12 +103,53 @@ tasks.register('startStandalone') {
 
 tasks.register('getServerVersion') {
     doLast {
-        new ByteArrayOutputStream().withStream { os ->
-            exec {
-                commandLine 'redis-server', '-v'
-                standardOutput = os
+        def detectedVersion
+        def output = new ByteArrayOutputStream()
+
+        // Helper method to find the full path of a command
+        def findFullPath = { command ->
+            def pathOutput = new ByteArrayOutputStream()
+            try {
+                exec {
+                    commandLine 'which', command  // Use 'where' for Windows
+                    standardOutput = pathOutput
+                }
+                return pathOutput.toString().trim()
+            } catch (Exception e) {
+                println "Failed to find path for ${command}: ${e.message}"
+                return ""
+            }
+        }
+
+        // Get full paths
+        def valkeyPath = findFullPath('valkey-server')
+        def redisPath = findFullPath('redis-server')
+
+        def tryGetVersion = { serverPath ->
+            try {
+                exec {
+                    commandLine serverPath, '-v'
+                    standardOutput = output
+                }
+                return output.toString()
+            } catch (Exception e) {
+                println "Failed to execute ${serverPath}: ${e.message}"
+                return ""
             }
-            serverVersion = extractServerVersion(os.toString())
+        }
+
+        // Try valkey-server first, then redis-server if it fails
+        def versionOutput = tryGetVersion(valkeyPath)
+        if (versionOutput.isEmpty() && !redisPath.isEmpty()) {
+            versionOutput = tryGetVersion(redisPath)
+        }
+
+        if (!versionOutput.isEmpty()) {
+            detectedVersion = extractServerVersion(versionOutput)
+            println "Detected server version: ${detectedVersion}"
+            serverVersion = detectedVersion
+        } else {
+            throw new GradleException("Failed to retrieve the server version.")
         }
     }
 }

diff --git a/java/integTest/src/test/java/glide/SharedCommandTests.java b/java/integTest/src/test/java/glide/SharedCommandTests.java
@@ -13774,9 +13774,14 @@ public void sscan(BaseClient client) {
         assertDeepEquals(new String[] {}, result[resultCollectionIndex]);
 
         // Negative cursor
-        result = client.sscan(key1, "-1").get();
-        assertEquals(initialCursor, result[resultCursorIndex]);
-        assertDeepEquals(new String[] {}, result[resultCollectionIndex]);
+        if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
+            ExecutionException executionException =
+                    assertThrows(ExecutionException.class, () -> client.sscan(key1, "-1").get());
+        } else {
+            result = client.sscan(key1, "-1").get();
+            assertEquals(initialCursor, result[resultCursorIndex]);
+            assertDeepEquals(new String[] {}, result[resultCollectionIndex]);
+        }
 
         // Result contains the whole set
         assertEquals(charMembers.length, client.sadd(key1, charMembers).get());
@@ -13910,9 +13915,14 @@ public void sscan_binary(BaseClient client) {
         assertDeepEquals(new GlideString[] {}, result[resultCollectionIndex]);
 
         // Negative cursor
-        result = client.sscan(key1, gs("-1")).get();
-        assertEquals(initialCursor, gs(result[resultCursorIndex].toString()));
-        assertDeepEquals(new GlideString[] {}, result[resultCollectionIndex]);
+        if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
+            ExecutionException executionException =
+                    assertThrows(ExecutionException.class, () -> client.sscan(key1, gs("-1")).get());
+        } else {
+            result = client.sscan(key1, gs("-1")).get();
+            assertEquals(initialCursor, gs(result[resultCursorIndex].toString()));
+            assertDeepEquals(new GlideString[] {}, result[resultCollectionIndex]);
+        }
 
         // Result contains the whole set
         assertEquals(charMembers.length, client.sadd(key1, charMembers).get());
@@ -14059,9 +14069,14 @@ public void zscan(BaseClient client) {
         assertDeepEquals(new String[] {}, result[resultCollectionIndex]);
 
         // Negative cursor
-        result = client.zscan(key1, "-1").get();
-        assertEquals(initialCursor, result[resultCursorIndex]);
-        assertDeepEquals(new String[] {}, result[resultCollectionIndex]);
+        if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
+            ExecutionException executionException =
+                    assertThrows(ExecutionException.class, () -> client.zscan(key1, "-1").get());
+        } else {
+            result = client.zscan(key1, "-1").get();
+            assertEquals(initialCursor, result[resultCursorIndex]);
+            assertDeepEquals(new String[] {}, result[resultCollectionIndex]);
+        }
 
         // Result contains the whole set
         assertEquals(charMembers.length, client.zadd(key1, charMap).get());
@@ -14240,9 +14255,14 @@ public void zscan_binary(BaseClient client) {
         assertDeepEquals(new GlideString[] {}, result[resultCollectionIndex]);
 
         // Negative cursor
-        result = client.zscan(key1, gs("-1")).get();
-        assertEquals(initialCursor, gs(result[resultCursorIndex].toString()));
-        assertDeepEquals(new GlideString[] {}, result[resultCollectionIndex]);
+        if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
+            ExecutionException executionException =
+                    assertThrows(ExecutionException.class, () -> client.zscan(key1, gs("-1")).get());
+        } else {
+            result = client.zscan(key1, gs("-1")).get();
+            assertEquals(initialCursor, gs(result[resultCursorIndex].toString()));
+            assertDeepEquals(new GlideString[] {}, result[resultCollectionIndex]);
+        }
 
         // Result contains the whole set
         assertEquals(charMembers.length, client.zadd(key1.toString(), charMap_strings).get());
@@ -14425,9 +14445,14 @@ public void hscan(BaseClient client) {
         assertDeepEquals(new String[] {}, result[resultCollectionIndex]);
 
         // Negative cursor
-        result = client.hscan(key1, "-1").get();
-        assertEquals(initialCursor, result[resultCursorIndex]);
-        assertDeepEquals(new String[] {}, result[resultCollectionIndex]);
+        if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
+            ExecutionException executionException =
+                    assertThrows(ExecutionException.class, () -> client.hscan(key1, "-1").get());
+        } else {
+            result = client.hscan(key1, "-1").get();
+            assertEquals(initialCursor, result[resultCursorIndex]);
+            assertDeepEquals(new String[] {}, result[resultCollectionIndex]);
+        }
 
         // Result contains the whole set
         assertEquals(charMembers.length, client.hset(key1, charMap).get());
@@ -14589,9 +14614,14 @@ public void hscan_binary(BaseClient client) {
         assertDeepEquals(new GlideString[] {}, result[resultCollectionIndex]);
 
         // Negative cursor
-        result = client.hscan(key1, gs("-1")).get();
-        assertEquals(initialCursor, gs(result[resultCursorIndex].toString()));
-        assertDeepEquals(new GlideString[] {}, result[resultCollectionIndex]);
+        if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
+            ExecutionException executionException =
+                    assertThrows(ExecutionException.class, () -> client.hscan(key1, gs("-1")).get());
+        } else {
+            result = client.hscan(key1, gs("-1")).get();
+            assertEquals(initialCursor, gs(result[resultCursorIndex].toString()));
+            assertDeepEquals(new GlideString[] {}, result[resultCollectionIndex]);
+        }
 
         // Result contains the whole set
         assertEquals(charMembers.length, client.hset(key1, charMap).get());

diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java
@@ -1051,15 +1051,19 @@ public void flushall() {
         assertEquals(OK, clusterClient.flushall(ASYNC, route).get());
 
         var replicaRoute = new SlotKeyRoute("key", REPLICA);
-        // command should fail on a replica, because it is read-only
-        ExecutionException executionException =
-                assertThrows(ExecutionException.class, () -> clusterClient.flushall(replicaRoute).get());
-        assertInstanceOf(RequestException.class, executionException.getCause());
-        assertTrue(
-                executionException
-                        .getMessage()
-                        .toLowerCase()
-                        .contains("can't write against a read only replica"));
+        if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
+            assertEquals(OK, clusterClient.flushall(route).get());
+        } else {
+            // command should fail on a replica, because it is read-only
+            ExecutionException executionException =
+                    assertThrows(ExecutionException.class, () -> clusterClient.flushall(replicaRoute).get());
+            assertInstanceOf(RequestException.class, executionException.getCause());
+            assertTrue(
+                    executionException
+                            .getMessage()
+                            .toLowerCase()
+                            .contains("can't write against a read only replica"));
+        }
     }
 
     // TODO: add a binary version of this test

diff --git a/java/integTest/src/test/java/glide/standalone/CommandTests.java b/java/integTest/src/test/java/glide/standalone/CommandTests.java
@@ -1535,9 +1535,14 @@ public void scan() {
         assertDeepEquals(new String[] {}, emptyResult[resultCollectionIndex]);
 
         // Negative cursor
-        Object[] negativeResult = regularClient.scan("-1").get();
-        assertEquals(initialCursor, negativeResult[resultCursorIndex]);
-        assertDeepEquals(new String[] {}, negativeResult[resultCollectionIndex]);
+        if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
+            ExecutionException executionException =
+                    assertThrows(ExecutionException.class, () -> regularClient.scan("-1").get());
+        } else {
+            Object[] negativeResult = regularClient.scan("-1").get();
+            assertEquals(initialCursor, negativeResult[resultCursorIndex]);
+            assertDeepEquals(new String[] {}, negativeResult[resultCollectionIndex]);
+        }
 
         // Add keys to the database using mset
         regularClient.mset(keys).get();
@@ -1589,9 +1594,14 @@ public void scan_binary() {
         assertDeepEquals(new String[] {}, emptyResult[resultCollectionIndex]);
 
         // Negative cursor
-        Object[] negativeResult = regularClient.scan(gs("-1")).get();
-        assertEquals(initialCursor, negativeResult[resultCursorIndex]);
-        assertDeepEquals(new String[] {}, negativeResult[resultCollectionIndex]);
+        if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
+            ExecutionException executionException =
+                    assertThrows(ExecutionException.class, () -> regularClient.scan(gs("-1")).get());
+        } else {
+            Object[] negativeResult = regularClient.scan(gs("-1")).get();
+            assertEquals(initialCursor, negativeResult[resultCursorIndex]);
+            assertDeepEquals(new String[] {}, negativeResult[resultCollectionIndex]);
+        }
 
         // Add keys to the database using mset
         regularClient.msetBinary(keys).get();
@@ -1652,9 +1662,16 @@ public void scan_with_options() {
         assertDeepEquals(new String[] {}, emptyResult[resultCollectionIndex]);
 
         // Negative cursor
-        Object[] negativeResult = regularClient.scan("-1", options).get();
-        assertEquals(initialCursor, negativeResult[resultCursorIndex]);
-        assertDeepEquals(new String[] {}, negativeResult[resultCollectionIndex]);
+        if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
+            final ScanOptions finalOptions = options;
+            ExecutionException executionException =
+                    assertThrows(
+                            ExecutionException.class, () -> regularClient.scan("-1", finalOptions).get());
+        } else {
+            Object[] negativeResult = regularClient.scan("-1", options).get();
+            assertEquals(initialCursor, negativeResult[resultCursorIndex]);
+            assertDeepEquals(new String[] {}, negativeResult[resultCollectionIndex]);
+        }
 
         // scan for strings by match pattern:
         options =
@@ -1734,9 +1751,16 @@ public void scan_binary_with_options() {
         assertDeepEquals(new String[] {}, emptyResult[resultCollectionIndex]);
 
         // Negative cursor
-        Object[] negativeResult = regularClient.scan(gs("-1"), options).get();
-        assertEquals(initialCursor, negativeResult[resultCursorIndex]);
-        assertDeepEquals(new String[] {}, negativeResult[resultCollectionIndex]);
+        if (SERVER_VERSION.isGreaterThanOrEqualTo("7.9.0")) {
+            final ScanOptions finalOptions = options;
+            ExecutionException executionException =
+                    assertThrows(
+                            ExecutionException.class, () -> regularClient.scan(gs("-1"), finalOptions).get());
+        } else {
+            Object[] negativeResult = regularClient.scan(gs("-1"), options).get();
+            assertEquals(initialCursor, negativeResult[resultCursorIndex]);
+            assertDeepEquals(new String[] {}, negativeResult[resultCollectionIndex]);
+        }
 
         // scan for strings by match pattern:
         options =