[GitHub] [tinkerpop] jorgebay commented on a change in pull request #1539: TINKERPOP-2679 update javascript driver to support stream processing
jorgebay commented on a change in pull request #1539:
URL: https://github.com/apache/tinkerpop/pull/1539#discussion_r787817459
##
File path: docs/src/reference/gremlin-variants.asciidoc
##
@@ -1721,6 +1721,32 @@ IMPORTANT: The preferred method for setting a
per-request timeout for scripts is
with bytecode may try `g.with(EVALUATION_TIMEOUT, 500)` within a script.
Scripts with multiple traversals and multiple
timeouts will be interpreted as a sum of all timeouts identified in the script
for that request.
+
+ Processing results as they are returned from the Gremlin server
+
+
+The Gremlin JavaScript driver maintains a WebSocket connection to the Gremlin
server and receives messages according to the `batchSize` parameter on the per
request settings or the `resultIterationBatchSize` value configured for the
Gremlin server. When submitting scripts the default behavior is to wait for the
entire result set to be returned from a query before allowing any processing on
the result set.
+
+The following examples assume that you have 100 vertices in your graph.
+
+[source,javascript]
+
+const result = await client.submit("g.V()");
+console.log(result.toArray()); // 100 - all the vertices in your graph
+
+
+When working with larger result sets it may be beneficial for memory
management to process each chunk of data as it is returned from the gremlin
server. The Gremlin JavaScript driver can accept an optional callback to run on
each chunk of data returned.
+
+[source,javascript]
+
+
+await client.submit("g.V()", {}, { batchSize: 25 }, (data) => {
Review comment:
I think an async iterator is harder to implement in this case given that
the tinkerpop protocol doesn't allow to fetch the next page (it will continue
to serve the next "partial content").
I think an event emitter (emitter of result items) or a stream fit better in
this case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [tinkerpop] jorgebay commented on a change in pull request #1539: TINKERPOP-2679 update javascript driver to support stream processing
jorgebay commented on a change in pull request #1539:
URL: https://github.com/apache/tinkerpop/pull/1539#discussion_r787809380
##
File path: docs/src/reference/gremlin-variants.asciidoc
##
@@ -1721,6 +1721,32 @@ IMPORTANT: The preferred method for setting a
per-request timeout for scripts is
with bytecode may try `g.with(EVALUATION_TIMEOUT, 500)` within a script.
Scripts with multiple traversals and multiple
timeouts will be interpreted as a sum of all timeouts identified in the script
for that request.
+
+ Processing results as they are returned from the Gremlin server
+
+
+The Gremlin JavaScript driver maintains a WebSocket connection to the Gremlin
server and receives messages according to the `batchSize` parameter on the per
request settings or the `resultIterationBatchSize` value configured for the
Gremlin server. When submitting scripts the default behavior is to wait for the
entire result set to be returned from a query before allowing any processing on
the result set.
+
+The following examples assume that you have 100 vertices in your graph.
+
+[source,javascript]
+
+const result = await client.submit("g.V()");
+console.log(result.toArray()); // 100 - all the vertices in your graph
+
+
+When working with larger result sets it may be beneficial for memory
management to process each chunk of data as it is returned from the gremlin
server. The Gremlin JavaScript driver can accept an optional callback to run on
each chunk of data returned.
+
+[source,javascript]
+
+
+await client.submit("g.V()", {}, { batchSize: 25 }, (data) => {
Review comment:
I think from the user POV, having a thing (`ResultSet`, `Stream`, ...)
that represents the result of the execution is more familiar than having 1 per
"batch". The batch is a low level thing.
There are several db client libraries that use this approach (blocking
iterators in other languages, async iterators in node.js, streams or
callbacks). I [worked on the paging api for
Cassandra](https://github.com/datastax/nodejs-driver/tree/master/doc/features/paging)
that uses several of these approaches (see [here for
example](https://github.com/datastax/nodejs-driver#row-streaming-and-pipes)),
but these are popular patterns across node.js db client libraries (see
[mysql](https://github.com/mysqljs/mysql#streaming-query-rows))
I think that we should return an object that fires events or a stream of
result items, we should clearly signal when all the data has been received or
when there was an error. If we use callbacks, I think we shouldn't mix it with
promises to get the error.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [tinkerpop] tkolanko commented on a change in pull request #1539: TINKERPOP-2679 update javascript driver to support stream processing
tkolanko commented on a change in pull request #1539:
URL: https://github.com/apache/tinkerpop/pull/1539#discussion_r787801589
##
File path: docs/src/reference/gremlin-variants.asciidoc
##
@@ -1721,6 +1721,32 @@ IMPORTANT: The preferred method for setting a
per-request timeout for scripts is
with bytecode may try `g.with(EVALUATION_TIMEOUT, 500)` within a script.
Scripts with multiple traversals and multiple
timeouts will be interpreted as a sum of all timeouts identified in the script
for that request.
+
+ Processing results as they are returned from the Gremlin server
+
+
+The Gremlin JavaScript driver maintains a WebSocket connection to the Gremlin
server and receives messages according to the `batchSize` parameter on the per
request settings or the `resultIterationBatchSize` value configured for the
Gremlin server. When submitting scripts the default behavior is to wait for the
entire result set to be returned from a query before allowing any processing on
the result set.
+
+The following examples assume that you have 100 vertices in your graph.
+
+[source,javascript]
+
+const result = await client.submit("g.V()");
+console.log(result.toArray()); // 100 - all the vertices in your graph
+
+
+When working with larger result sets it may be beneficial for memory
management to process each chunk of data as it is returned from the gremlin
server. The Gremlin JavaScript driver can accept an optional callback to run on
each chunk of data returned.
+
+[source,javascript]
+
+
+await client.submit("g.V()", {}, { batchSize: 25 }, (data) => {
Review comment:
ah ok, let me take a look at how complex it would be to add an async
iterator to websocket messages. Through a quick google search there are
[some](https://github.com/alanshaw/it-ws)
[libraries](https://github.com/alanshaw/it-ws) but they don't seem to be
maintained
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [tinkerpop] jorgebay commented on a change in pull request #1539: TINKERPOP-2679 update javascript driver to support stream processing
jorgebay commented on a change in pull request #1539:
URL: https://github.com/apache/tinkerpop/pull/1539#discussion_r787798256
##
File path: docs/src/reference/gremlin-variants.asciidoc
##
@@ -1721,6 +1721,32 @@ IMPORTANT: The preferred method for setting a
per-request timeout for scripts is
with bytecode may try `g.with(EVALUATION_TIMEOUT, 500)` within a script.
Scripts with multiple traversals and multiple
timeouts will be interpreted as a sum of all timeouts identified in the script
for that request.
+
+ Processing results as they are returned from the Gremlin server
+
+
+The Gremlin JavaScript driver maintains a WebSocket connection to the Gremlin
server and receives messages according to the `batchSize` parameter on the per
request settings or the `resultIterationBatchSize` value configured for the
Gremlin server. When submitting scripts the default behavior is to wait for the
entire result set to be returned from a query before allowing any processing on
the result set.
+
+The following examples assume that you have 100 vertices in your graph.
+
+[source,javascript]
+
+const result = await client.submit("g.V()");
+console.log(result.toArray()); // 100 - all the vertices in your graph
+
+
+When working with larger result sets it may be beneficial for memory
management to process each chunk of data as it is returned from the gremlin
server. The Gremlin JavaScript driver can accept an optional callback to run on
each chunk of data returned.
+
+[source,javascript]
+
+
+await client.submit("g.V()", {}, { batchSize: 25 }, (data) => {
Review comment:
> we don't operate on traversals
I used the variable name `traversal` for the string script containing the
traversal, I edited the code example for clarity.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [tinkerpop] jorgebay commented on a change in pull request #1539: TINKERPOP-2679 update javascript driver to support stream processing
jorgebay commented on a change in pull request #1539:
URL: https://github.com/apache/tinkerpop/pull/1539#discussion_r787504326
##
File path: docs/src/reference/gremlin-variants.asciidoc
##
@@ -1721,6 +1721,32 @@ IMPORTANT: The preferred method for setting a
per-request timeout for scripts is
with bytecode may try `g.with(EVALUATION_TIMEOUT, 500)` within a script.
Scripts with multiple traversals and multiple
timeouts will be interpreted as a sum of all timeouts identified in the script
for that request.
+
+ Processing results as they are returned from the Gremlin server
+
+
+The Gremlin JavaScript driver maintains a WebSocket connection to the Gremlin
server and receives messages according to the `batchSize` parameter on the per
request settings or the `resultIterationBatchSize` value configured for the
Gremlin server. When submitting scripts the default behavior is to wait for the
entire result set to be returned from a query before allowing any processing on
the result set.
+
+The following examples assume that you have 100 vertices in your graph.
+
+[source,javascript]
+
+const result = await client.submit("g.V()");
+console.log(result.toArray()); // 100 - all the vertices in your graph
+
+
+When working with larger result sets it may be beneficial for memory
management to process each chunk of data as it is returned from the gremlin
server. The Gremlin JavaScript driver can accept an optional callback to run on
each chunk of data returned.
+
+[source,javascript]
+
+
+await client.submit("g.V()", {}, { batchSize: 25 }, (data) => {
Review comment:
I think mixing promises and callbacks in the same API can be confusing
and prone to misuse.
I think we should expose a different method for "streaming" or grouping into
smaller sets of results, for example with async iterables:
```javascript
const stream = client.stream('g.V()');
for await (const item of stream) {
statement
}
```
or regular callbacks
```javascript
client.forEach('g.V()', item => {
// called for each item
}, err => {
// called at the end or when there's an error
});
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [tinkerpop] tkolanko commented on a change in pull request #1539: TINKERPOP-2679 update javascript driver to support stream processing
tkolanko commented on a change in pull request #1539:
URL: https://github.com/apache/tinkerpop/pull/1539#discussion_r78779
##
File path: docs/src/reference/gremlin-variants.asciidoc
##
@@ -1721,6 +1721,32 @@ IMPORTANT: The preferred method for setting a
per-request timeout for scripts is
with bytecode may try `g.with(EVALUATION_TIMEOUT, 500)` within a script.
Scripts with multiple traversals and multiple
timeouts will be interpreted as a sum of all timeouts identified in the script
for that request.
+
+ Processing results as they are returned from the Gremlin server
+
+
+The Gremlin JavaScript driver maintains a WebSocket connection to the Gremlin
server and receives messages according to the `batchSize` parameter on the per
request settings or the `resultIterationBatchSize` value configured for the
Gremlin server. When submitting scripts the default behavior is to wait for the
entire result set to be returned from a query before allowing any processing on
the result set.
+
+The following examples assume that you have 100 vertices in your graph.
+
+[source,javascript]
+
+const result = await client.submit("g.V()");
+console.log(result.toArray()); // 100 - all the vertices in your graph
+
+
+When working with larger result sets it may be beneficial for memory
management to process each chunk of data as it is returned from the gremlin
server. The Gremlin JavaScript driver can accept an optional callback to run on
each chunk of data returned.
+
+[source,javascript]
+
+
+await client.submit("g.V()", {}, { batchSize: 25 }, (data) => {
Review comment:
Our use case might be different, we don't operate on traverals but
through submitting scripts. We have a front end user interface where users can
enter queries with auto complete, intellisense etc etc along with some options.
The frontend POSTS the query and options to our backend which submits the
script, parses the result set and returns a result to the frontend.
I went with this approach so we could so something like
```js
import {parseResultSet} from './parser';
...code to handle parsing the POST request and extracting what we need...
const output = {...} // object that holds our parsed data
try {
await client.submit(query, bindings, options, (data) =>
parseResultSet(data, output))
} catch(e) {
...additional error handling/clean up...
throw new Error(e);
}
return res.json(output))
```
This seemed like an easier opt in path than trying to handle async iteration
on websocket messages
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [tinkerpop] tkolanko commented on a change in pull request #1539: TINKERPOP-2679 update javascript driver to support stream processing
tkolanko commented on a change in pull request #1539:
URL: https://github.com/apache/tinkerpop/pull/1539#discussion_r787791909
##
File path:
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/connection.js
##
@@ -290,13 +296,31 @@ class Connection extends EventEmitter {
}
switch (response.status.code) {
case responseStatusCode.noContent:
+if (this._onDataMessageHandlers[response.requestId]) {
+ this._onDataMessageHandlers[response.requestId](
+new ResultSet(utils.emptyArray, response.status.attributes)
+ );
+}
this._clearHandler(response.requestId);
return handler.callback(null, new ResultSet(utils.emptyArray,
response.status.attributes));
case responseStatusCode.partialContent:
-handler.result = handler.result || [];
-handler.result.push.apply(handler.result, response.result.data);
+if (this._onDataMessageHandlers[response.requestId]) {
+ this._onDataMessageHandlers[response.requestId](
+new ResultSet(response.result.data, response.status.attributes)
Review comment:
That would mean different parsing would have to take place between
partial content and final parsing/if the total results were less than the batch
size.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[tinkerpop] branch dependabot/nuget/gremlin-dotnet/3.5-dev/System.Text.Json-6.0.1 updated (e9fd16b -> 27ae854)
This is an automated email from the ASF dual-hosted git repository. github-bot pushed a change to branch dependabot/nuget/gremlin-dotnet/3.5-dev/System.Text.Json-6.0.1 in repository https://gitbox.apache.org/repos/asf/tinkerpop.git. discard e9fd16b Bump System.Text.Json from 5.0.2 to 6.0.1 in /gremlin-dotnet add ebcfdf3 fix: add missing comma in python example add 5db8aad Bump to Netty 4.1.72 CTR add 459067c Merge branch '3.4-dev' into 3.5-dev add 18bdf7d change aiohttp requirements due to vulnerability issue at 3.7.4 add 11f2b6e Merge branch 'pr-1519' into 3.5-dev add 6f45069 Source: [1] Added transaction profile to pom.xml [2] Made DriverRemoteConnection latch parameters so they can be reused to create a subsequent session [3] Added logging throughout the driver [4] Added commit and rollback to DriverRemoteConnection [5] Added some logging to receive message [6] Added transaction support to RemoteConnection [7] Added bytecode support to Session processor [8] Fixed bug in aiohttp transport layer that popped up when it was not shutdown properl [...] add dd796be [1] Fixed TEST_TRANSACTION environment variable [2] Enabling transaction tests in GitHub actions add 2594cf5 [1] Added session support to string messages. This was unintentionally removed. add d1e3abd Added submitAsync in Client and DriverRemoteConnection with deprecated message Fixed missing session close in Client Switched info to debug log for heavy spam messages Added gremlin-variant remote transaction documentation for gremlin-python Added release documentation for remote transactions in gremlin-python add 5a0a835 Changed logic for disabling transactions within tests. add fee9056 Merge branch 'pr-1515' into 3.5-dev add a65c01b Added an `AnonymizingTypeTranslator` for use with `GroovyTranslator` which strips PII (anonymizes any String, Numeric, Date, Timestamp, or UUID data). add 070e168 Test case cleanup. add 922c6e1 Test case cleanup. add e7e2fd2 PR feedback add d2835bb Harden testing around driver integration test CTR add b51c97e Added transaction testing to Gremlin Server for CI CTR add f6f7ddc Merge branch '3.4-dev' into 3.5-dev add 9ab9433 Changed seconds to minutes for timeout - oops CTR add 5f239f8 Merge branch '3.4-dev' into 3.5-dev add 039fc15 Fixed minor nits in changelog CTR add a2d020e Merge branch '3.5-dev' into TINKERPOP-2666 add d8e2794 TINKERPOP-2667 Allowed fold()/addAll to merge Map objects add 014ba55 Merge branch 'TINKERPOP-2667' into 3.5-dev add e0412a6 Reduced resources consumed by gremlin server integration tests add 84cfe8f Merge branch '3.4-dev' into 3.5-dev add c0605b3 Minor fix to session test CTR add 8b0d2d1 TINKERPOP-2626 Added explicit closed state to DefaultTraversal add 579a65e Merge branch '3.4-dev' into 3.5-dev add 5e48db8 TINKERPOP-2670 Fixed javadoc on jdk11. add 511539b Added user-friendly console message and fixed possible console remote leak add c76df03 Minor fix to test CTR add befc7c2 Merge branch '3.4-dev' into 3.5-dev add a191930 TINKERPOP-2671 Added tx() support in gremlin-language add d263a3b Merge branch 'TINKERPOP-2671' into 3.5-dev add 4859617 After #1534 the exceptions seemed to shift on the merge to 3.5.x add af57b3e Minor fix to code example to get docs generating CTR add e501c75 Fixed NOTICE dates and netty version CTR add 58c81f3 TinkerPop release 3.4.13 add 0413cd5 Merge branch '3.4-dev' into 3.5-dev add a4bc39b TinkerPop 3.5.2 release add 4e648bf Update gremlint dependencies to get rid of vulnerabilities CTR add 134180f Update gremlint website dependencies to get rid of vulnerabilities CTR add 93d7109 gremlin-javascript: Tune Writer/Reader abstraction border add ec78520 Merge pull request #1549 from ihoro/gremlin-javascript-tune-writer-reader-abstraction-border add 8e4c226 TINKERPOP-2651 Update to .NET 6 add 71a977f Merge branch 'TINKERPOP-2651' into 3.5-dev add 27ae854 Bump System.Text.Json from 5.0.2 to 6.0.1 in /gremlin-dotnet This update added new revisions after undoing existing revisions. That is to say, some revisions that were in the old version of the branch are not in the new version. This situation occurs when a user --force pushes a change and generates a repository containing something like this: * -- * -- B -- O -- O -- O (e9fd16b) \ N -- N -- N refs/heads/dependabot/nuget/gremlin-dotnet/3.5-dev/System.Text.Json-6.0.1 (27ae854) You should already have received notification emails for all of the O revisions, and so the following emails describe only the N revisions from the common base, B. Any revisions marked "omit" are not gone; other references still refer to them. Any revisions marked "discard" are gone forever. No new
[tinkerpop] branch dependabot/nuget/gremlin-dotnet/3.5-dev/System.Text.Json-6.0.1 created (now e9fd16b)
This is an automated email from the ASF dual-hosted git repository. github-bot pushed a change to branch dependabot/nuget/gremlin-dotnet/3.5-dev/System.Text.Json-6.0.1 in repository https://gitbox.apache.org/repos/asf/tinkerpop.git. at e9fd16b Bump System.Text.Json from 5.0.2 to 6.0.1 in /gremlin-dotnet No new revisions were added by this update.
[GitHub] [tinkerpop] FlorianHockmann commented on pull request #1521: Bump System.Text.Json from 5.0.2 to 6.0.1 in /gremlin-dotnet
FlorianHockmann commented on pull request #1521: URL: https://github.com/apache/tinkerpop/pull/1521#issuecomment-1016503349 @dependabot reopen -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [tinkerpop] FlorianHockmann merged pull request #1538: TINKERPOP-2651 Update to .NET 6
FlorianHockmann merged pull request #1538: URL: https://github.com/apache/tinkerpop/pull/1538 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[tinkerpop] branch 3.5-dev updated (ec78520 -> 71a977f)
This is an automated email from the ASF dual-hosted git repository. florianhockmann pushed a change to branch 3.5-dev in repository https://gitbox.apache.org/repos/asf/tinkerpop.git. from ec78520 Merge pull request #1549 from ihoro/gremlin-javascript-tune-writer-reader-abstraction-border add 8e4c226 TINKERPOP-2651 Update to .NET 6 add 71a977f Merge branch 'TINKERPOP-2651' into 3.5-dev No new revisions were added by this update. Summary of changes: .github/workflows/build-test.yml | 4 ++-- docker/Dockerfile | 2 +- docs/src/dev/developer/development-environment.asciidoc | 4 ++-- gremlin-dotnet/src/Gremlin.Net.Template/Gremlin.Net.Template.csproj | 2 +- gremlin-dotnet/src/pom.xml| 2 +- .../test/Gremlin.Net.IntegrationTest/Gherkin/GherkinTestRunner.cs | 2 +- .../Gremlin.Net.IntegrationTest/Gremlin.Net.IntegrationTest.csproj| 2 +- .../Gremlin.Net.Template.IntegrationTest.csproj | 2 +- gremlin-dotnet/test/Gremlin.Net.UnitTest/Gremlin.Net.UnitTest.csproj | 2 +- gremlin-dotnet/test/pom.xml | 2 +- 10 files changed, 12 insertions(+), 12 deletions(-)
[tinkerpop] 01/01: Merge branch '3.5-dev'
This is an automated email from the ASF dual-hosted git repository. florianhockmann pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tinkerpop.git commit f48fc35b59700d558fd06e60d90e5691b85574ba Merge: fe74928 71a977f Author: Florian Hockmann AuthorDate: Wed Jan 19 15:06:30 2022 +0100 Merge branch '3.5-dev' .github/workflows/build-test.yml | 4 +- docker/Dockerfile | 2 +- .../dev/developer/development-environment.asciidoc | 4 +- .../Gremlin.Net.Template.csproj| 2 +- gremlin-dotnet/src/pom.xml | 2 +- .../Gherkin/GherkinTestRunner.cs | 2 +- .../Gremlin.Net.IntegrationTest.csproj | 2 +- .../Gremlin.Net.Template.IntegrationTest.csproj| 2 +- .../Gremlin.Net.UnitTest.csproj| 2 +- gremlin-dotnet/test/pom.xml| 2 +- .../gremlin-javascript/lib/driver/connection.js| 66 ++ .../lib/structure/io/graph-serializer.js | 46 +++ 12 files changed, 74 insertions(+), 62 deletions(-)
[tinkerpop] branch master updated (fe74928 -> f48fc35)
This is an automated email from the ASF dual-hosted git repository. florianhockmann pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/tinkerpop.git. from fe74928 Merge pull request #1545 from mikepersonick/TINKERPOP-2641 add 4e648bf Update gremlint dependencies to get rid of vulnerabilities CTR add 134180f Update gremlint website dependencies to get rid of vulnerabilities CTR add 93d7109 gremlin-javascript: Tune Writer/Reader abstraction border add ec78520 Merge pull request #1549 from ihoro/gremlin-javascript-tune-writer-reader-abstraction-border add 8e4c226 TINKERPOP-2651 Update to .NET 6 add 71a977f Merge branch 'TINKERPOP-2651' into 3.5-dev new f48fc35 Merge branch '3.5-dev' The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: .github/workflows/build-test.yml | 4 +- docker/Dockerfile | 2 +- .../dev/developer/development-environment.asciidoc | 4 +- .../Gremlin.Net.Template.csproj| 2 +- gremlin-dotnet/src/pom.xml | 2 +- .../Gherkin/GherkinTestRunner.cs | 2 +- .../Gremlin.Net.IntegrationTest.csproj | 2 +- .../Gremlin.Net.Template.IntegrationTest.csproj| 2 +- .../Gremlin.Net.UnitTest.csproj| 2 +- gremlin-dotnet/test/pom.xml| 2 +- .../gremlin-javascript/lib/driver/connection.js| 66 ++ .../lib/structure/io/graph-serializer.js | 46 +++ 12 files changed, 74 insertions(+), 62 deletions(-)
[GitHub] [tinkerpop] FlorianHockmann commented on a change in pull request #1551: Fixing .NET test failures and adding Gherkin/.NET support for VertexProperty.
FlorianHockmann commented on a change in pull request #1551: URL: https://github.com/apache/tinkerpop/pull/1551#discussion_r787737917 ## File path: gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Process/Remote/RemoteStrategyTests.cs ## @@ -1,4 +1,4 @@ -#region License Review comment: I at least remember that we had some problems with this already in the past. I think some IDE (maybe Visual Studio) added the BOM byte to files which resulted in problems for others. I think we should remove it everywhere. We could add an `.editorconfig` where we specify the charset as `utf-8` for all `*.cs` files. Most editors / IDEs support `.editorconfig` (a plugin might be necessary though) which should ensure that the BOM byte doesn't get added back. We can also use a GitHub action to enforce that the `.editorconfig` is honored. This issue contains more details: editorconfig/editorconfig#297. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [tinkerpop] mikepersonick commented on a change in pull request #1551: Fixing .NET test failures and adding Gherkin/.NET support for VertexProperty.
mikepersonick commented on a change in pull request #1551:
URL: https://github.com/apache/tinkerpop/pull/1551#discussion_r787730911
##
File path:
gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/ScenarioData.cs
##
@@ -133,6 +137,70 @@ public ScenarioData(IMessageSerializer messageSerializer)
}
}
+private static IDictionary
GetVertexProperties(GraphTraversalSource g)
+{
+try
+{
+/*
+ * This closure will turn a VertexProperty into a triple
string of the form:
+ * "vertexName-propKey->propVal"
+ *
+ * It will also take care of wrapping propVal in the
appropriate Numeric format. We must do this in
+ * case a Vertex has multiple properties with the same key and
number value but in different numeric
+ * type spaces (rare, but possible, and presumably something
we may want to write tests around).
+ */
+string groovy = @"
+{ vp ->
+ def result = vp.element().value('name') + '-' +
vp.key() + '->'
+ def value = vp.value()
+ def type = ''
+ switch(value) {
Review comment:
It was the same in Java. Now some poor soul has to do the same in
Javascript and Python...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [tinkerpop] mikepersonick commented on a change in pull request #1551: Fixing .NET test failures and adding Gherkin/.NET support for VertexProperty.
mikepersonick commented on a change in pull request #1551:
URL: https://github.com/apache/tinkerpop/pull/1551#discussion_r787730911
##
File path:
gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/ScenarioData.cs
##
@@ -133,6 +137,70 @@ public ScenarioData(IMessageSerializer messageSerializer)
}
}
+private static IDictionary
GetVertexProperties(GraphTraversalSource g)
+{
+try
+{
+/*
+ * This closure will turn a VertexProperty into a triple
string of the form:
+ * "vertexName-propKey->propVal"
+ *
+ * It will also take care of wrapping propVal in the
appropriate Numeric format. We must do this in
+ * case a Vertex has multiple properties with the same key and
number value but in different numeric
+ * type spaces (rare, but possible, and presumably something
we may want to write tests around).
+ */
+string groovy = @"
+{ vp ->
+ def result = vp.element().value('name') + '-' +
vp.key() + '->'
+ def value = vp.value()
+ def type = ''
+ switch(value) {
Review comment:
Some poor soul has to do the same in Javascript and Python...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [tinkerpop] mikepersonick commented on a change in pull request #1551: Fixing .NET test failures and adding Gherkin/.NET support for VertexProperty.
mikepersonick commented on a change in pull request #1551: URL: https://github.com/apache/tinkerpop/pull/1551#discussion_r787727496 ## File path: gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Process/Remote/RemoteStrategyTests.cs ## @@ -1,4 +1,4 @@ -#region License Review comment: Is this a known thing? Rider was not able to compile those source files for me bc of it. Is there an IDE setting that can fix it? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [tinkerpop] FlorianHockmann commented on a change in pull request #1551: Fixing .NET test failures and adding Gherkin/.NET support for VertexProperty.
FlorianHockmann commented on a change in pull request #1551:
URL: https://github.com/apache/tinkerpop/pull/1551#discussion_r787486627
##
File path:
gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/ScenarioData.cs
##
@@ -133,6 +137,70 @@ public ScenarioData(IMessageSerializer messageSerializer)
}
}
+private static IDictionary
GetVertexProperties(GraphTraversalSource g)
+{
+try
+{
+/*
+ * This closure will turn a VertexProperty into a triple
string of the form:
+ * "vertexName-propKey->propVal"
+ *
+ * It will also take care of wrapping propVal in the
appropriate Numeric format. We must do this in
+ * case a Vertex has multiple properties with the same key and
number value but in different numeric
+ * type spaces (rare, but possible, and presumably something
we may want to write tests around).
+ */
+string groovy = @"
Review comment:
(nitpick) Really not important, but you can most of the time just use
`var` instead of the concrete type of a variable as that can be interfered from
its initialization. So, `var groovy = [...]` here.
This makes it a bit nicer to read in my opinion.
##
File path:
gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/ScenarioData.cs
##
@@ -133,6 +137,70 @@ public ScenarioData(IMessageSerializer messageSerializer)
}
}
+private static IDictionary
GetVertexProperties(GraphTraversalSource g)
+{
+try
+{
+/*
+ * This closure will turn a VertexProperty into a triple
string of the form:
+ * "vertexName-propKey->propVal"
+ *
+ * It will also take care of wrapping propVal in the
appropriate Numeric format. We must do this in
+ * case a Vertex has multiple properties with the same key and
number value but in different numeric
+ * type spaces (rare, but possible, and presumably something
we may want to write tests around).
+ */
+string groovy = @"
+{ vp ->
+ def result = vp.element().value('name') + '-' +
vp.key() + '->'
+ def value = vp.value()
+ def type = ''
+ switch(value) {
Review comment:
Wow, a lot of effort to get these scenarios to work in .NET
##
File path:
gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Process/Remote/RemoteStrategyTests.cs
##
@@ -1,4 +1,4 @@
-#region License
Review comment:
Hmm, the good old BOM byte. Maybe we should add an `.editorconfig` to
prevent these from being added back again?
(Not suggesting that something like that should be added as part of this PR,
just something that came to my mind while seeing this here.)
##
File path:
gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/ScenarioData.cs
##
@@ -133,6 +137,70 @@ public ScenarioData(IMessageSerializer messageSerializer)
}
}
+private static IDictionary
GetVertexProperties(GraphTraversalSource g)
+{
+try
+{
+/*
+ * This closure will turn a VertexProperty into a triple
string of the form:
+ * "vertexName-propKey->propVal"
+ *
+ * It will also take care of wrapping propVal in the
appropriate Numeric format. We must do this in
+ * case a Vertex has multiple properties with the same key and
number value but in different numeric
+ * type spaces (rare, but possible, and presumably something
we may want to write tests around).
+ */
+string groovy = @"
+{ vp ->
+ def result = vp.element().value('name') + '-' +
vp.key() + '->'
+ def value = vp.value()
+ def type = ''
+ switch(value) {
+case { !(it instanceof Number) }:
+ return result + value
+case Byte:
+ type = 'b'
+ break
+case Short:
+ type = 's'
+ break
+case Integer:
+ type = 'i'
+ break
+case Long:
+ type = 'l'
+ break
+case Float:
+ type = 'f'
+ break
+case
[GitHub] [tinkerpop] spmallette commented on pull request #1551: Fixing .NET test failures and adding Gherkin/.NET support for VertexProperty.
spmallette commented on pull request #1551: URL: https://github.com/apache/tinkerpop/pull/1551#issuecomment-1016418256 Adding `VertexProperty` to the test suite is most excellent - thanks. VOTE +1 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [tinkerpop] FlorianHockmann commented on pull request #1551: Fixing .NET test failures and adding Gherkin/.NET support for VertexProperty.
FlorianHockmann commented on pull request #1551: URL: https://github.com/apache/tinkerpop/pull/1551#issuecomment-1016310534 Thanks for this great contribution to Gremlin.NET, @mikepersonick! I just added two nitpick comments about C# style, but this is good to go from my side with or without addressing them. VOTE +1 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [tinkerpop] jorgebay commented on a change in pull request #1539: TINKERPOP-2679 update javascript driver to support stream processing
jorgebay commented on a change in pull request #1539:
URL: https://github.com/apache/tinkerpop/pull/1539#discussion_r787504326
##
File path: docs/src/reference/gremlin-variants.asciidoc
##
@@ -1721,6 +1721,32 @@ IMPORTANT: The preferred method for setting a
per-request timeout for scripts is
with bytecode may try `g.with(EVALUATION_TIMEOUT, 500)` within a script.
Scripts with multiple traversals and multiple
timeouts will be interpreted as a sum of all timeouts identified in the script
for that request.
+
+ Processing results as they are returned from the Gremlin server
+
+
+The Gremlin JavaScript driver maintains a WebSocket connection to the Gremlin
server and receives messages according to the `batchSize` parameter on the per
request settings or the `resultIterationBatchSize` value configured for the
Gremlin server. When submitting scripts the default behavior is to wait for the
entire result set to be returned from a query before allowing any processing on
the result set.
+
+The following examples assume that you have 100 vertices in your graph.
+
+[source,javascript]
+
+const result = await client.submit("g.V()");
+console.log(result.toArray()); // 100 - all the vertices in your graph
+
+
+When working with larger result sets it may be beneficial for memory
management to process each chunk of data as it is returned from the gremlin
server. The Gremlin JavaScript driver can accept an optional callback to run on
each chunk of data returned.
+
+[source,javascript]
+
+
+await client.submit("g.V()", {}, { batchSize: 25 }, (data) => {
Review comment:
I think mixing promises and callbacks in the same API can be confusing
and prone to misuse.
I think we should expose a different method for "streaming" or grouping into
smaller sets of results, for example with async iterables:
```javascript
const stream = client.stream(traversal);
for await (const item of stream) {
statement
}
```
or regular callbacks
```javascript
client.forEach(traversal, item => {
// called for each item
}, err => {
// called at the end or when there's an error
});
```
##
File path:
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/connection.js
##
@@ -290,13 +296,31 @@ class Connection extends EventEmitter {
}
switch (response.status.code) {
case responseStatusCode.noContent:
+if (this._onDataMessageHandlers[response.requestId]) {
+ this._onDataMessageHandlers[response.requestId](
+new ResultSet(utils.emptyArray, response.status.attributes)
+ );
+}
this._clearHandler(response.requestId);
return handler.callback(null, new ResultSet(utils.emptyArray,
response.status.attributes));
case responseStatusCode.partialContent:
-handler.result = handler.result || [];
-handler.result.push.apply(handler.result, response.result.data);
+if (this._onDataMessageHandlers[response.requestId]) {
+ this._onDataMessageHandlers[response.requestId](
+new ResultSet(response.result.data, response.status.attributes)
Review comment:
Maybe instead of having 4 resultsets, the user wants to access each
individual item.
##
File path: docs/src/reference/gremlin-variants.asciidoc
##
@@ -1721,6 +1721,32 @@ IMPORTANT: The preferred method for setting a
per-request timeout for scripts is
with bytecode may try `g.with(EVALUATION_TIMEOUT, 500)` within a script.
Scripts with multiple traversals and multiple
timeouts will be interpreted as a sum of all timeouts identified in the script
for that request.
+
+ Processing results as they are returned from the Gremlin server
+
+
+The Gremlin JavaScript driver maintains a WebSocket connection to the Gremlin
server and receives messages according to the `batchSize` parameter on the per
request settings or the `resultIterationBatchSize` value configured for the
Gremlin server. When submitting scripts the default behavior is to wait for the
entire result set to be returned from a query before allowing any processing on
the result set.
Review comment:
Nice way to introduce the change
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[tinkerpop] branch 3.5-dev updated: gremlin-javascript: Tune Writer/Reader abstraction border
This is an automated email from the ASF dual-hosted git repository.
jorgebg 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 93d7109 gremlin-javascript: Tune Writer/Reader abstraction border
new ec78520 Merge pull request #1549 from
ihoro/gremlin-javascript-tune-writer-reader-abstraction-border
93d7109 is described below
commit 93d7109153d5f44cd67e437a8db2277335ef3973
Author: Igor Ostapenko
AuthorDate: Tue Jan 11 23:55:40 2022 +0200
gremlin-javascript: Tune Writer/Reader abstraction border
---
.../gremlin-javascript/lib/driver/connection.js| 66 ++
.../lib/structure/io/graph-serializer.js | 46 +++
2 files changed, 62 insertions(+), 50 deletions(-)
diff --git
a/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/connection.js
b/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/connection.js
index cf46475..a726b21 100644
---
a/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/connection.js
+++
b/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/connection.js
@@ -29,7 +29,6 @@ const utils = require('../utils');
const serializer = require('../structure/io/graph-serializer');
const ResultSet = require('./result-set');
const ResponseError = require('./response-error');
-const Bytecode = require('../process/bytecode');
const responseStatusCode = {
success: 200,
@@ -92,7 +91,8 @@ class Connection extends EventEmitter {
this._pingInterval = null;
this._pongTimeout = null;
-this._header = String.fromCharCode(this.mimeType.length) + this.mimeType;
+this._header = String.fromCharCode(this.mimeType.length) + this.mimeType;
// TODO: what if mimeType.length > 255
+this._header_buf = Buffer.from(this._header);
this.isOpen = false;
this.traversalSource = options.traversalSource || 'g';
this._authenticator = options.authenticator;
@@ -169,7 +169,19 @@ class Connection extends EventEmitter {
};
}
- const message = Buffer.from(this._header +
JSON.stringify(this._getRequest(rid, op, args, processor)));
+ const request = {
+'requestId': rid,
+'op': op || 'bytecode',
+// if using op eval need to ensure processor stays unset if caller
didn't set it.
+'processor': (!processor && op !== 'eval') ? 'traversal' : processor,
+'args': args || {},
+ };
+
+ const request_buf = this._writer.writeRequest(request);
+ const message = Buffer.concat(
+[this._header_buf, request_buf],
+this._header_buf.length + request_buf.length
+ );
this._ws.send(message);
}));
}
@@ -186,26 +198,6 @@ class Connection extends EventEmitter {
: new serializer.GraphSONWriter();
}
- _getRequest(id, op, args, processor) {
-if (args) {
- args = this._adaptArgs(args, true);
-} else {
- args = {};
-}
-
-if (args['gremlin'] instanceof Bytecode) {
- args['gremlin'] = this._writer.adaptObject(args['gremlin']);
-}
-
-return ({
- 'requestId': { '@type': 'g:UUID', '@value': id },
- 'op': op || 'bytecode',
- // if using op eval need to ensure processor stays unset if caller
didn't set it.
- 'processor': (!processor && op !== 'eval') ? 'traversal' : processor,
- 'args': args
-});
- }
-
_pingHeartbeat() {
if (this._pingInterval) {
@@ -247,7 +239,7 @@ class Connection extends EventEmitter {
}
_handleMessage(data) {
-const response = this._reader.read(JSON.parse(data.toString()));
+const response = this._reader.readResponse(data);
if (response.requestId === null || response.requestId === undefined) {
// There was a serialization issue on the server that prevented the
parsing of the request id
// We invoke any of the pending handlers with an error
@@ -337,32 +329,6 @@ class Connection extends EventEmitter {
}
/**
- * Takes the given args map and ensures all arguments are passed through to
_write.adaptObject
- * @param {Object} args Map of arguments to process.
- * @param {Boolean} protocolLevel Determines whether it's a protocol level
binding.
- * @returns {Object}
- * @private
- */
- _adaptArgs(args, protocolLevel) {
-if (args instanceof Object) {
- let newObj = {};
- Object.keys(args).forEach((key) => {
-// bindings key (at the protocol-level needs special handling. without
this, it wraps the generated Map
-// in another map for types like EnumValue. Could be a nicer way to do
this but for now it's solving the
-// problem with script submission of non JSON native types
-if (protocolLevel && key === 'bindings')
- newObj[key] = this._adaptArgs(args[key], false);
-else
- newObj[key] = this._writer.adaptObject(args[key]);
-
[GitHub] [tinkerpop] jorgebay merged pull request #1549: gremlin-javascript: Tune Writer/Reader abstraction border
jorgebay merged pull request #1549: URL: https://github.com/apache/tinkerpop/pull/1549 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
