Re: [PR] feat: `gremlin-javascript` browser support [tinkerpop]
tien commented on PR #2506: URL: https://github.com/apache/tinkerpop/pull/2506#issuecomment-1977996547 @kenhuuu thanks for this. I'll try to find another VOTE +1 if possible. Would be beneficial for me to get this in early because there's a work project I'm on that's currently using this. -- 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
Re: [PR] feat: `gremlin-javascript` browser support [tinkerpop]
kenhuuu commented on PR #2506: URL: https://github.com/apache/tinkerpop/pull/2506#issuecomment-1977977554 VOTE +1. Thanks again for your willingness to contribute. It is much appreciated. I'm not sure if you are familiar with our policy but it takes a VOTE+1 from three different committers to allow a PR to get merged immediately. Otherwise, if you get at least one of them (you have two right now), then the PR can be merged one week after the first VOTE +1. So at the latest this PR will be eligible for merging on Monday of next week. -- 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
Re: [PR] feat: `gremlin-javascript` browser support [tinkerpop]
kenhuuu commented on PR #2506: URL: https://github.com/apache/tinkerpop/pull/2506#issuecomment-1977973238 > @kenhuuu regarding the user agent header [used](https://tinkerpop.apache.org/docs/current/dev/provider/#_graph_driver_provider_requirements) by Gremlin server > > This is likely a separate discussion, but `User-Agent` header might not be the best way to identify client supported GLV version. Since browser environment for example doesn't allow you to set or modify this for web socket connection. > > What might be better is using the [`Sec-WebSocket-Protocol`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Protocol_upgrade_mechanism#sec-websocket-protocol) header instead. Which is designed to specify the sub-protocol requested by client & can be set in the browser by doing: > > ```js > new WebSocket("ws://localhost:8182", "glv3.7"); > ``` > > The sub-protocol can even be registered [here](https://www.iana.org/assignments/websocket/websocket.xml#subprotocol-name) if needed :)) Thanks for the information. As I mentioned above, we'll be moving to HTTP only in 4.x so its probably best to keep the 3.x WebSocket behavior the same for now. -- 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
Re: [PR] feat: `gremlin-javascript` browser support [tinkerpop]
kenhuuu commented on code in PR #2506: URL: https://github.com/apache/tinkerpop/pull/2506#discussion_r1512135739 ## gremlin-javascript/src/main/javascript/gremlin-javascript/lib/utils.js: ## @@ -112,8 +95,36 @@ exports.getUserAgentHeader = function getUserAgentHeader() { return 'User-Agent'; }; -const userAgent = generateUserAgent(); +exports.getUserAgent = async () => { Review Comment: I see. Although it would have been best for this to work the same between the web version and the Node version, I still think its worth getting this PR in so I think this can be overlooked. By the way, in the master branch, we will be removing WebSocket support and transitioning to HTTP so this won't be an issue there anyway. -- 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
Re: [PR] feat: `gremlin-javascript` browser support [tinkerpop]
tien commented on PR #2506: URL: https://github.com/apache/tinkerpop/pull/2506#issuecomment-1977944006 @kenhuuu regarding the user agent header [used](https://tinkerpop.apache.org/docs/current/dev/provider/#_graph_driver_provider_requirements) by Gremlin server This is likely a separate discussion, but `User-Agent` header might not be the best way to identify client supported GLV version. Since browser environment for example doesn't allow you to set or modify this for web socket connection. What might be better is using the [`Sec-WebSocket-Protocol`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Protocol_upgrade_mechanism#sec-websocket-protocol) header instead. Which is designed to specify the sub-protocol requested by client & can be set in the browser by doing: ```js new WebSocket("ws://localhost:8182", "glv3.7.0"); ``` -- 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
Re: [PR] feat: `gremlin-javascript` browser support [tinkerpop]
tien commented on code in PR #2506: URL: https://github.com/apache/tinkerpop/pull/2506#discussion_r1512022240 ## gremlin-javascript/src/main/javascript/gremlin-javascript/lib/utils.js: ## @@ -112,8 +95,36 @@ exports.getUserAgentHeader = function getUserAgentHeader() { return 'User-Agent'; }; -const userAgent = generateUserAgent(); +exports.getUserAgent = async () => { Review Comment: Yes, but unfortunately to my knowledge, Web API Web Socket doesn't allow you to set custom headers. -- 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
Re: [PR] feat: `gremlin-javascript` browser support [tinkerpop]
kenhuuu commented on code in PR #2506: URL: https://github.com/apache/tinkerpop/pull/2506#discussion_r1511974766 ## gremlin-javascript/src/main/javascript/gremlin-javascript/lib/utils.js: ## @@ -112,8 +95,36 @@ exports.getUserAgentHeader = function getUserAgentHeader() { return 'User-Agent'; }; -const userAgent = generateUserAgent(); +exports.getUserAgent = async () => { Review Comment: Please excuse my lack of knowledge in JavaScript, but it seems like this changes the user agent header that we will be sending to the server. If this is the case, we will want to change this to be more like how the NodeUserAgent works. Our specification for the user agent can be found at https://tinkerpop.apache.org/docs/current/dev/provider/#_graph_driver_provider_requirements . Notably the form should be of "[Application Name] [GLV Name].[Version] [Language Runtime Version] [OS].[Version] [CPU Architecture]". The reason this is important is because the server will likely start using this value in the future to enable/disable features based on the GLV's version. -- 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
Re: [PR] Avoiding expensive hash computation in filter ranking strategy [tinkerpop]
kenhuuu commented on PR #2504: URL: https://github.com/apache/tinkerpop/pull/2504#issuecomment-1977710799 I don't completely follow the changes being made here but they seem to be well tested so my VOTE is +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
Re: [PR] feat: `gremlin-javascript` browser support [tinkerpop]
tien commented on PR #2506: URL: https://github.com/apache/tinkerpop/pull/2506#issuecomment-1977625588 > VOTE +1 Thanks , just made a small change, updating the browser example package.json description. -- 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
Re: [PR] feat: `gremlin-javascript` browser support [tinkerpop]
vkagamlyk commented on PR #2506: URL: https://github.com/apache/tinkerpop/pull/2506#issuecomment-1977529989 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
Re: [PR] feat: `gremlin-javascript` browser support [tinkerpop]
vkagamlyk commented on PR #2506: URL: https://github.com/apache/tinkerpop/pull/2506#issuecomment-1977275669 > > @tien I got an error `index.js:22 Uncaught SyntaxError: The requested module '/@fs/C:/_Projects/tinkerpop-tien/gremlin-javascript/src/main/javascript/gremlin-javascript/index.js' does not provide an export named 'default' (at index.js:22:8)` in Chrome. > > Is this from running the example? To run the example, you need to do this at `tinkerpop/gremlin-javascript/examples/browser`: > > ```shell > yarn > yarn dev > ``` ahh, I tried with `npm`. Looks nice ![image](https://github.com/apache/tinkerpop/assets/7605999/4b6a36c1-f6a5-4e6f-a9eb-e73043f4ff76) -- 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
Re: [PR] feat: `gremlin-javascript` browser support [tinkerpop]
tien commented on PR #2506: URL: https://github.com/apache/tinkerpop/pull/2506#issuecomment-1977244785 > @tien I got an error `index.js:22 Uncaught SyntaxError: The requested module '/@fs/C:/_Projects/tinkerpop-tien/gremlin-javascript/src/main/javascript/gremlin-javascript/index.js' does not provide an export named 'default' (at index.js:22:8)` in Chrome. Is this from running the example? To run the example, you need to do this at `tinkerpop/gremlin-javascript/examples/browser`: ```sh yarn yarn dev ``` -- 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) 03/03: Merge branch '3.7-dev'
This is an automated email from the ASF dual-hosted git repository. spmallette pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tinkerpop.git commit d52b0b1217ae1425b327e2841b3bb5928dc5c95a Merge: 03e9f24f06 06af2ec6d5 Author: Stephen Mallette AuthorDate: Mon Mar 4 12:55:15 2024 -0500 Merge branch '3.7-dev' CHANGELOG.asciidoc| 1 + .../traversal/strategy/optimization/PathRetractionStrategy.java | 4 2 files changed, 5 insertions(+)
(tinkerpop) branch 3.6-dev updated (1efe40b288 -> beb1a5f35d)
This is an automated email from the ASF dual-hosted git repository. spmallette pushed a change to branch 3.6-dev in repository https://gitbox.apache.org/repos/asf/tinkerpop.git from 1efe40b288 TINKERPOP-3054 Fix requestId Deserialization in `gremlin-python` (#2494) add beb1a5f35d Improve performance of PathRetractionStrategy No new revisions were added by this update. Summary of changes: CHANGELOG.asciidoc| 1 + .../traversal/strategy/optimization/PathRetractionStrategy.java | 4 2 files changed, 5 insertions(+)
(tinkerpop) branch master updated (03e9f24f06 -> d52b0b1217)
This is an automated email from the ASF dual-hosted git repository. spmallette pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/tinkerpop.git from 03e9f24f06 Merge branch '3.7-dev' new beb1a5f35d Improve performance of PathRetractionStrategy new 06af2ec6d5 Merge branch '3.6-dev' into 3.7-dev new d52b0b1217 Merge branch '3.7-dev' The 3 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: CHANGELOG.asciidoc| 1 + .../traversal/strategy/optimization/PathRetractionStrategy.java | 4 2 files changed, 5 insertions(+)
(tinkerpop) 02/03: Merge branch '3.6-dev' into 3.7-dev
This is an automated email from the ASF dual-hosted git repository. spmallette pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tinkerpop.git commit 06af2ec6d58d5727ebc9c70f85bc4a199273d9ce Merge: cc9b3cf02c beb1a5f35d Author: Stephen Mallette AuthorDate: Mon Mar 4 12:55:03 2024 -0500 Merge branch '3.6-dev' into 3.7-dev CHANGELOG.asciidoc| 1 + .../traversal/strategy/optimization/PathRetractionStrategy.java | 4 2 files changed, 5 insertions(+)
(tinkerpop) branch 3.7-dev updated (cc9b3cf02c -> 06af2ec6d5)
This is an automated email from the ASF dual-hosted git repository. spmallette pushed a change to branch 3.7-dev in repository https://gitbox.apache.org/repos/asf/tinkerpop.git from cc9b3cf02c Merge branch '3.6-dev' into 3.7-dev add beb1a5f35d Improve performance of PathRetractionStrategy add 06af2ec6d5 Merge branch '3.6-dev' into 3.7-dev No new revisions were added by this update. Summary of changes: CHANGELOG.asciidoc| 1 + .../traversal/strategy/optimization/PathRetractionStrategy.java | 4 2 files changed, 5 insertions(+)
(tinkerpop) 01/03: Improve performance of PathRetractionStrategy
This is an automated email from the ASF dual-hosted git repository. spmallette pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tinkerpop.git commit beb1a5f35d27e0e76d3a75f2cceb6546b503b297 Author: Stephen Mallette AuthorDate: Fri Mar 1 09:25:09 2024 -0500 Improve performance of PathRetractionStrategy Helpful for traversals with lots of children where labels don't need to propagate. CTR --- CHANGELOG.asciidoc| 1 + .../traversal/strategy/optimization/PathRetractionStrategy.java | 4 2 files changed, 5 insertions(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 6f2e3d63c3..e6f71e13bc 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -25,6 +25,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima * Fixed a bug in Gremlin.Net for .NET 8 that led to exceptions: `InvalidOperationException: Enumeration has not started. Call MoveNext.` * Fixed message requestId serialization in `gremlin-python`. +* Improved performance of `PathRetractionStrategy` for traversals that carry many children, but don't hold many labels to propogate. * Fixed bug in bytecode translation of `g.tx().commit()` and `g.tx().rollback()` in all languages. * Improved error message from `JavaTranslator` by including exception source. * Added missing `short` serialization (`gx:Int16`) to GraphSONV2 and GraphSONV3 in `gremlin-python`. diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategy.java index cd29b28820..fc4e845797 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategy.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategy.java @@ -249,6 +249,10 @@ public final class PathRetractionStrategy extends AbstractTraversalStrategy keepLabels, final List> children) { +// if there are no labels to keep, then there no need to iterate all the children because we won't be +// adding anything PathProcessor keepLabels - avoids the added recursion +if (keepLabels.isEmpty()) return; + for (final Traversal.Admin child : children) { TraversalHelper.applyTraversalRecursively(trav -> addLabels(trav, keepLabels), child); }
Re: [PR] feat: `gremlin-javascript` browser support [tinkerpop]
vkagamlyk commented on PR #2506: URL: https://github.com/apache/tinkerpop/pull/2506#issuecomment-1977112339 @tien I got an error `index.js:22 Uncaught SyntaxError: The requested module '/@fs/C:/_Projects/tinkerpop-tien/gremlin-javascript/src/main/javascript/gremlin-javascript/index.js' does not provide an export named 'default' (at index.js:22:8)` in Chrome. -- 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/maven/3.6-dev/ch.qos.logback-logback-classic-1.5.1 deleted (was 5303556b48)
This is an automated email from the ASF dual-hosted git repository. github-bot pushed a change to branch dependabot/maven/3.6-dev/ch.qos.logback-logback-classic-1.5.1 in repository https://gitbox.apache.org/repos/asf/tinkerpop.git was 5303556b48 Bump ch.qos.logback:logback-classic from 1.2.11 to 1.5.1 The revisions that were on this branch are still contained in other references; therefore, this change does not discard any commits from the repository.
Re: [PR] Bump ch.qos.logback:logback-classic from 1.2.11 to 1.5.1 [tinkerpop]
dependabot[bot] closed pull request #2508: Bump ch.qos.logback:logback-classic from 1.2.11 to 1.5.1 URL: https://github.com/apache/tinkerpop/pull/2508 -- 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
Re: [PR] Bump ch.qos.logback:logback-classic from 1.2.11 to 1.5.1 [tinkerpop]
dependabot[bot] commented on PR #2508: URL: https://github.com/apache/tinkerpop/pull/2508#issuecomment-1976773833 Superseded by #2512. -- 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/maven/3.6-dev/ch.qos.logback-logback-classic-1.5.3 created (now c474cd7301)
This is an automated email from the ASF dual-hosted git repository. github-bot pushed a change to branch dependabot/maven/3.6-dev/ch.qos.logback-logback-classic-1.5.3 in repository https://gitbox.apache.org/repos/asf/tinkerpop.git at c474cd7301 Bump ch.qos.logback:logback-classic from 1.2.11 to 1.5.3 No new revisions were added by this update.
[PR] Bump ch.qos.logback:logback-classic from 1.2.11 to 1.5.3 [tinkerpop]
dependabot[bot] opened a new pull request, #2512: URL: https://github.com/apache/tinkerpop/pull/2512 Bumps [ch.qos.logback:logback-classic](https://github.com/qos-ch/logback) from 1.2.11 to 1.5.3. Commits https://github.com/qos-ch/logback/commit/f2d8a1a30a5b53a906fa5fa2dd8e7fd70911;>f2d8a1a prepare release 1.5.3 https://github.com/qos-ch/logback/commit/99ccca7986b5fb0fd029946e92623c0ac4d3868e;>99ccca7 fix /issues/785 https://github.com/qos-ch/logback/commit/df7c7c55e94289af4ee92324d4359ee131cab4e8;>df7c7c5 prepare work on 1.5.3-SNAPSHOT https://github.com/qos-ch/logback/commit/baf5d705612d058e2b3323b648cd4592ae586337;>baf5d70 prepare release 1.5.2 https://github.com/qos-ch/logback/commit/6998e81db1f2cb1b863f57c5e32b7d3a12459e0d;>6998e81 add NoAutoStartUtil.shouldBeStarted method https://github.com/qos-ch/logback/commit/40bc1c225da6df2d4234eb4eb2fd18ceee816a4e;>40bc1c2 export tyler related packages https://github.com/qos-ch/logback/commit/0094ba31e8f728554c1bb3182aca7c2ac61f13ed;>0094ba3 start work on 1.5.2-SNAPSHOT https://github.com/qos-ch/logback/commit/88fd31c639ba6cad5e8c11bf980690f04e27eb1e;>88fd31c prepare release 1.5.1 https://github.com/qos-ch/logback/commit/231e9bdcb3bf9e79fd028a4ca89fa8616d9d7551;>231e9bd more internal changes and refactorings https://github.com/qos-ch/logback/commit/8f3a30422fbec60715aa14abc5321bc5c76e9b5d;>8f3a304 more fine tuning of propertyModelHandler and co Additional commits viewable in https://github.com/qos-ch/logback/compare/v_1.2.11...v_1.5.3;>compare view [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=ch.qos.logback:logback-classic=maven=1.2.11=1.5.3)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- Dependabot commands and options You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) -- 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