Skip to content

[TINKERPOP-2841] Test and Fix Per-Request Settings in Go GLV#1939

Merged
vkagamlyk merged 1 commit intoapache:masterfrom
Bit-Quill:cole/go-per-request-settings
Feb 9, 2023
Merged

[TINKERPOP-2841] Test and Fix Per-Request Settings in Go GLV#1939
vkagamlyk merged 1 commit intoapache:masterfrom
Bit-Quill:cole/go-per-request-settings

Conversation

@Cole-Greer
Copy link
Contributor

https://issues.apache.org/jira/browse/TINKERPOP-2841

In my testing, the go glv was not sending per-request options correctly to the server. Go was passing request options inside the gremlin script bindings map instead of within the args portion of the request message.

This PR re-implements the per-request settings by adding new Client.SubmitWithOptions() and DriverRemoteConnection.SubmitWithOptions() methods. Both original Submit() methods are unchanged in their behavior so this will not break any users.

This PR introduces a new RequestOptions struct and builder which is used to configure per-request settings and passed into the new SubmitWithOptions() methods.

@codecov-commenter
Copy link

codecov-commenter commented Jan 12, 2023

Codecov Report

Merging #1939 (c16da29) into master (3c46c4e) will increase coverage by 0.39%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #1939      +/-   ##
============================================
+ Coverage     68.58%   68.98%   +0.39%     
+ Complexity     9094     9089       -5     
============================================
  Files           854      830      -24     
  Lines         41188    37457    -3731     
  Branches       5598     5598              
============================================
- Hits          28250    25839    -2411     
+ Misses        10962     9808    -1154     
+ Partials       1976     1810     -166     
Impacted Files Coverage Δ
.../gremlin/driver/exception/ConnectionException.java 22.22% <0.00%> (-22.23%) ⬇️
...rg/apache/tinkerpop/gremlin/driver/Connection.java 53.75% <0.00%> (-6.25%) ⬇️
...java/org/apache/tinkerpop/gremlin/driver/Host.java 35.55% <0.00%> (-2.23%) ⬇️
...pache/tinkerpop/gremlin/driver/ConnectionPool.java 28.89% <0.00%> (-0.65%) ⬇️
...in/process/traversal/dsl/graph/GraphTraversal.java 90.86% <0.00%> (-0.33%) ⬇️
gremlin-go/driver/client.go
gremlin-go/driver/driverRemoteConnection.go
gremlin-go/driver/request.go
gremlin-go/driver/graphBinary.go
gremlin-go/driver/bytecode.go
... and 21 more

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

@Cole-Greer Cole-Greer force-pushed the cole/go-per-request-settings branch from 9d08d0d to 0e512cf Compare January 12, 2023 17:37
@xiazcy
Copy link
Contributor

xiazcy commented Feb 3, 2023

LGTM, thanks Cole! VOTE +1

Go was previously not sending per-request options correctly to the server.
Go was passing request options inside the gremlin script bindings map instead of
within the args portion of the request message.

This commit resolves this issue by encapsulating all per-request settings as well as
script bindings into a new RequestOptions struct which has an accompanying RequestOptionsBuilder.

This can now be passed in through new Client.SubmitWithOptions() and DriverRemoteConnection.SubmitWithOptions() methods.
Both original Submit() methods are unchanged in their behavior so this will not break any users.
@Cole-Greer Cole-Greer force-pushed the cole/go-per-request-settings branch from 0e512cf to c16da29 Compare February 9, 2023 17:32
@vkagamlyk
Copy link
Contributor

VOTE +1

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.

4 participants