You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by xi...@apache.org on 2022/12/21 00:28:13 UTC

[tinkerpop] branch 3.5-dev updated: Fix RequestId Override CTR

This is an automated email from the ASF dual-hosted git repository.

xiazcy pushed a commit to branch 3.5-dev
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git


The following commit(s) were added to refs/heads/3.5-dev by this push:
     new 5d230e6b3e Fix RequestId Override CTR
     new 34b98b3ec6 Merge pull request #1914 from Bit-Quill/cole/fix_request_id_override
5d230e6b3e is described below

commit 5d230e6b3e419c8d6b7b8dc9594095d945b07ccc
Author: Cole Greer <co...@bitquilltech.com>
AuthorDate: Mon Dec 19 12:08:49 2022 -0800

    Fix RequestId Override CTR
    
    Fixes a bug where python and go glv's were not using the correct request_id
    when overriden. This is detailed in TINKERPOP-2841 and TINKERPOP-2844. This
    commit is not sufficient to close those tickets as further investigation is
    needed to ensure the other per-request settings work as intended, this
    commit provides an immediate fix to the broken request_id functionality.
---
 CHANGELOG.asciidoc                                 |  1 +
 gremlin-go/driver/request.go                       | 13 +++++-
 gremlin-go/driver/request_test.go                  | 50 ++++++++++++++++++++++
 .../python/gremlin_python/driver/connection.py     |  2 +
 4 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 7e6cfbc2a5..aa6681bb9f 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -57,6 +57,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 * Changed mechanism for determining `Host` health which should make the driver more resilient to intermittent network failures.
 * Removed the delay for reconnecting to a potentially unhealthy `Host` only marking it as unavailable after that initial retry fails.
 * Prevented fast `NoHostAvailableException` in favor of more direct exceptions when borrowing connections from the `ConnectionPool`.
+* Fixed an issue in Go and Python GLVs where modifying per request settings to override request_id's was not working correctly.
 
 ==== Bugs
 
diff --git a/gremlin-go/driver/request.go b/gremlin-go/driver/request.go
index 931fe7cccd..304317d9a3 100644
--- a/gremlin-go/driver/request.go
+++ b/gremlin-go/driver/request.go
@@ -19,7 +19,10 @@ under the License.
 
 package gremlingo
 
-import "github.com/google/uuid"
+import (
+	"fmt"
+	"github.com/google/uuid"
+)
 
 // request represents a request to the server.
 type request struct {
@@ -47,11 +50,17 @@ func makeStringRequest(stringGremlin string, traversalSource string, sessionId s
 		newProcessor = sessionProcessor
 		newArgs["session"] = sessionId
 	}
+	requestId := uuid.New()
 	if len(bindings) > 0 {
 		newArgs["bindings"] = bindings[0]
+		customRequestId, err := uuid.Parse(fmt.Sprintf("%v", bindings[0]["requestId"]))
+		if err == nil {
+			requestId = customRequestId
+		}
 	}
+
 	return request{
-		requestID: uuid.New(),
+		requestID: requestId,
 		op:        stringOp,
 		processor: newProcessor,
 		args:      newArgs,
diff --git a/gremlin-go/driver/request_test.go b/gremlin-go/driver/request_test.go
new file mode 100644
index 0000000000..0e1d8433e8
--- /dev/null
+++ b/gremlin-go/driver/request_test.go
@@ -0,0 +1,50 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+package gremlingo
+
+import (
+	"fmt"
+	"github.com/google/uuid"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestRequest(t *testing.T) {
+	t.Run("Test makeStringRequest() with custom requestID", func(t *testing.T) {
+		requestId := fmt.Sprintf("%v", uuid.New())
+		r := makeStringRequest("g.V()", "g", "", map[string]interface{}{"requestId": requestId})
+		assert.Equal(t, requestId, fmt.Sprintf("%v", r.requestID))
+	})
+
+	t.Run("Test makeStringRequest() with no bindings", func(t *testing.T) {
+		r := makeStringRequest("g.V()", "g", "")
+		assert.NotNil(t, r.requestID)
+		assert.NotEqual(t, uuid.Nil, r.requestID)
+	})
+
+	t.Run("Test makeStringRequest() with custom evaluationTimeout", func(t *testing.T) {
+		r := makeStringRequest("g.V()", "g", "", map[string]interface{}{"evaluationTimeout": 1234})
+		assert.NotNil(t, r.requestID)
+		assert.NotEqual(t, uuid.Nil, r.requestID)
+		bindings := r.args["bindings"].(map[string]interface{})
+		assert.Equal(t, 1234, bindings["evaluationTimeout"])
+	})
+}
diff --git a/gremlin-python/src/main/python/gremlin_python/driver/connection.py b/gremlin-python/src/main/python/gremlin_python/driver/connection.py
index 92ebb2795d..51ff84696c 100644
--- a/gremlin-python/src/main/python/gremlin_python/driver/connection.py
+++ b/gremlin-python/src/main/python/gremlin_python/driver/connection.py
@@ -59,6 +59,8 @@ class Connection:
         if not self._inited:
             self.connect()
         request_id = str(uuid.uuid4())
+        if request_message.args.get("requestId"):
+            request_id = request_message.args.get("requestId")
         result_set = resultset.ResultSet(queue.Queue(), request_id)
         self._results[request_id] = result_set
         # Create write task