[jira] [Commented] (TINKERPOP-1793) addE() should allow dynamic edge labels
[ https://issues.apache.org/jira/browse/TINKERPOP-1793?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16183615#comment-16183615 ] ASF GitHub Bot commented on TINKERPOP-1793: --- Github user robertdale commented on the issue: https://github.com/apache/tinkerpop/pull/724 Nice. VOTE +1 > addE() should allow dynamic edge labels > --- > > Key: TINKERPOP-1793 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1793 > Project: TinkerPop > Issue Type: Improvement > Components: process >Reporter: Daniel Kuppitz >Assignee: Marko A. Rodriguez > > We can create vertices with dynamic labels, e.g: > {noformat} > ...addV().property(label, select("x"))... > {noformat} > This approach doesn't work for edges as we don't have a parameterless > {{addE}} overload. However, I think we can allow {{addE()}} and use > {{Edge.DEFAULT_LABEL}}, just like we do for vertices; then the aforementioned > approach would work. > As an aside: I would prefer {{addV(traversal)}} and {{addE(traversal)}} over > {{addX().property(label, ...)}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[GitHub] tinkerpop issue #724: TINKERPOP-1793: addE() should allow dynamic edge label...
Github user robertdale commented on the issue: https://github.com/apache/tinkerpop/pull/724 Nice. VOTE +1 ---
[jira] [Commented] (TINKERPOP-1793) addE() should allow dynamic edge labels
[ https://issues.apache.org/jira/browse/TINKERPOP-1793?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16183332#comment-16183332 ] ASF GitHub Bot commented on TINKERPOP-1793: --- Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/724 ``` [INFO] [INFO] BUILD SUCCESS [INFO] [INFO] Total time: 02:21 h [INFO] Finished at: 2017-09-27T14:16:02-06:00 [INFO] Final Memory: 170M/1515M [INFO] ``` > addE() should allow dynamic edge labels > --- > > Key: TINKERPOP-1793 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1793 > Project: TinkerPop > Issue Type: Improvement > Components: process >Reporter: Daniel Kuppitz >Assignee: Marko A. Rodriguez > > We can create vertices with dynamic labels, e.g: > {noformat} > ...addV().property(label, select("x"))... > {noformat} > This approach doesn't work for edges as we don't have a parameterless > {{addE}} overload. However, I think we can allow {{addE()}} and use > {{Edge.DEFAULT_LABEL}}, just like we do for vertices; then the aforementioned > approach would work. > As an aside: I would prefer {{addV(traversal)}} and {{addE(traversal)}} over > {{addX().property(label, ...)}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[GitHub] tinkerpop issue #724: TINKERPOP-1793: addE() should allow dynamic edge label...
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/724 ``` [INFO] [INFO] BUILD SUCCESS [INFO] [INFO] Total time: 02:21 h [INFO] Finished at: 2017-09-27T14:16:02-06:00 [INFO] Final Memory: 170M/1515M [INFO] ``` ---
[jira] [Commented] (TINKERPOP-1793) addE() should allow dynamic edge labels
[ https://issues.apache.org/jira/browse/TINKERPOP-1793?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16183115#comment-16183115 ] ASF GitHub Bot commented on TINKERPOP-1793: --- Github user dkuppitz commented on the issue: https://github.com/apache/tinkerpop/pull/724 Manual tests (see [SO](https://stackoverflow.com/questions/46363737/tinkerpop-gremlin-merge-vertices-and-edges)) worked like a charm. VOTE: +1 > addE() should allow dynamic edge labels > --- > > Key: TINKERPOP-1793 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1793 > Project: TinkerPop > Issue Type: Improvement > Components: process >Reporter: Daniel Kuppitz >Assignee: Marko A. Rodriguez > > We can create vertices with dynamic labels, e.g: > {noformat} > ...addV().property(label, select("x"))... > {noformat} > This approach doesn't work for edges as we don't have a parameterless > {{addE}} overload. However, I think we can allow {{addE()}} and use > {{Edge.DEFAULT_LABEL}}, just like we do for vertices; then the aforementioned > approach would work. > As an aside: I would prefer {{addV(traversal)}} and {{addE(traversal)}} over > {{addX().property(label, ...)}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[GitHub] tinkerpop issue #724: TINKERPOP-1793: addE() should allow dynamic edge label...
Github user dkuppitz commented on the issue: https://github.com/apache/tinkerpop/pull/724 Manual tests (see [SO](https://stackoverflow.com/questions/46363737/tinkerpop-gremlin-merge-vertices-and-edges)) worked like a charm. VOTE: +1 ---
[GitHub] tinkerpop pull request #724: TINKERPOP-1793: addE() should allow dynamic edg...
GitHub user okram opened a pull request: https://github.com/apache/tinkerpop/pull/724 TINKERPOP-1793: addE() should allow dynamic edge labels https://issues.apache.org/jira/browse/TINKERPOP-1793 There are a few changes here: 1. `addV(traversal)` and `addE(traversal)` added to both `GraphTraversal` and `GraphTraversalSource`. Thus, the label of the respectively created element can be dynamically determined. 2. Added the above methods as well as `addE(String)` to the traversal source of `GremlinDslProcessor`. The missing `addE(String)` method was a bug. 3. Changed `to()` and `from()` from taking a `Traversal` to `Traversal`. This reduces type coercion. Added various test cases to validate proper functioning. VOTE +1. You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1793 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/724.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #724 commit bd2c3b27059a50b87960a5bf270c5b9633bb8edc Author: Marko A. Rodriguez Date: 2017-09-27T16:05:16Z first push of working addV(traversal) and addE(traversal). I have only written one test case thus far. However, taking a break and pushing what I have. Need to write about 3 more test cases to be confident. commit f0dc7ff5d72e4ba8532326fe9b99c35b476f4b09 Author: Marko A. Rodriguez Date: 2017-09-27T16:55:56Z added GraphTraversalSource.addV(traversal) and GraphTraversalSource.addE(traversal). Added 2 more test cases. One more to go. Updated upgrade docs accordingly. commit 8ff6bdf44d6a3bc922723c0646b5ad39c3c80268 Author: Marko A. Rodriguez Date: 2017-09-27T17:36:53Z changed the typing of from() and to() to accept wildcard instead of E as the start. this reduces nasty type coersion. Added final addE() test case. Updated GremlinDSLProcessor to handle g.addE() and new addV()/addE() traversal methods accordingly. commit 1f76f1e9b1bb3bd980cd3b143228a2c324220477 Author: Marko A. Rodriguez Date: 2017-09-27T17:54:17Z updated upgrade docs. ---
[jira] [Commented] (TINKERPOP-1793) addE() should allow dynamic edge labels
[ https://issues.apache.org/jira/browse/TINKERPOP-1793?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16182988#comment-16182988 ] ASF GitHub Bot commented on TINKERPOP-1793: --- GitHub user okram opened a pull request: https://github.com/apache/tinkerpop/pull/724 TINKERPOP-1793: addE() should allow dynamic edge labels https://issues.apache.org/jira/browse/TINKERPOP-1793 There are a few changes here: 1. `addV(traversal)` and `addE(traversal)` added to both `GraphTraversal` and `GraphTraversalSource`. Thus, the label of the respectively created element can be dynamically determined. 2. Added the above methods as well as `addE(String)` to the traversal source of `GremlinDslProcessor`. The missing `addE(String)` method was a bug. 3. Changed `to()` and `from()` from taking a `Traversal` to `Traversal`. This reduces type coercion. Added various test cases to validate proper functioning. VOTE +1. You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1793 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/724.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #724 commit bd2c3b27059a50b87960a5bf270c5b9633bb8edc Author: Marko A. Rodriguez Date: 2017-09-27T16:05:16Z first push of working addV(traversal) and addE(traversal). I have only written one test case thus far. However, taking a break and pushing what I have. Need to write about 3 more test cases to be confident. commit f0dc7ff5d72e4ba8532326fe9b99c35b476f4b09 Author: Marko A. Rodriguez Date: 2017-09-27T16:55:56Z added GraphTraversalSource.addV(traversal) and GraphTraversalSource.addE(traversal). Added 2 more test cases. One more to go. Updated upgrade docs accordingly. commit 8ff6bdf44d6a3bc922723c0646b5ad39c3c80268 Author: Marko A. Rodriguez Date: 2017-09-27T17:36:53Z changed the typing of from() and to() to accept wildcard instead of E as the start. this reduces nasty type coersion. Added final addE() test case. Updated GremlinDSLProcessor to handle g.addE() and new addV()/addE() traversal methods accordingly. commit 1f76f1e9b1bb3bd980cd3b143228a2c324220477 Author: Marko A. Rodriguez Date: 2017-09-27T17:54:17Z updated upgrade docs. > addE() should allow dynamic edge labels > --- > > Key: TINKERPOP-1793 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1793 > Project: TinkerPop > Issue Type: Improvement > Components: process >Reporter: Daniel Kuppitz >Assignee: Marko A. Rodriguez > > We can create vertices with dynamic labels, e.g: > {noformat} > ...addV().property(label, select("x"))... > {noformat} > This approach doesn't work for edges as we don't have a parameterless > {{addE}} overload. However, I think we can allow {{addE()}} and use > {{Edge.DEFAULT_LABEL}}, just like we do for vertices; then the aforementioned > approach would work. > As an aside: I would prefer {{addV(traversal)}} and {{addE(traversal)}} over > {{addX().property(label, ...)}}. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (TINKERPOP-1752) Gremlin.Net: Generate completely type-safe methods
[ https://issues.apache.org/jira/browse/TINKERPOP-1752?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16182717#comment-16182717 ] ASF GitHub Bot commented on TINKERPOP-1752: --- Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/712#discussion_r141376043 --- Diff: gremlin-dotnet/src/Gremlin.Net/Process/Traversal/Bytecode.cs --- @@ -79,7 +85,77 @@ public void AddSource(string sourceName, params object[] args) /// The traversal method arguments. public void AddStep(string stepName, params object[] args) { -StepInstructions.Add(new Instruction(stepName, args)); +StepInstructions.Add(new Instruction(stepName, FlattenArguments(args))); +Bindings.Clear(); --- End diff -- That's a good point. So we should probably support lambdas. To be honest, I haven't really looked into what would be necessary to support lambdas. If I'm not mistaken then we can easily support Python and Groovy lambdas when we add a dedicated GraphSON serializer for them and probably a class that wraps the lambda together with its language. Then the steps that take a lambda would simply accept an object of that class in Gremlin.Net. I can create a separate ticket for this when we agree that we want to support them. When we want to support lambdas then we should also support `Bindings`. That leaves us with the question of whether we find a better implementation that doesn't suffer from the concurrency problems @jorgebay mentioned or if the current implementation in this pull request is acceptable. Apart from that, shouldn't we clarify the section in the documentation about bindings? It currently states for example [for gremlin-python](http://tinkerpop.apache.org/docs/current/reference/#_bindings): > If a traversal is going to be executed repeatedly, but with different parameters, then bindings should be used. because: > translation and compilation can be skipped as the previously compiled version should be cached which is only true when a lambda is used if I understood it correctly. > Gremlin.Net: Generate completely type-safe methods > -- > > Key: TINKERPOP-1752 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1752 > Project: TinkerPop > Issue Type: Improvement > Components: dotnet >Affects Versions: 3.2.5 >Reporter: Florian Hockmann >Priority: Minor > > Currently the generated traversal methods in Gremlin.Net take {{params > object[] args}} as an argument which allows the user to provide an arbitrary > number of arguments with any type. While this makes the generation rather > simple, it doesn't tell the user which arguments are actually valid so users > can submit completely invalid traversals like: > {code} > g.V(1).AddE(1234, "invalidArgument2").Next() > {code} > Type-safe methods could also use the original argument names to tell users > something about what kind of values the methods expect. Consider for example > the following method signatures for the C# step {{AddE}} that are basically a > 1:1 representation of the original Java {{addE}} step: > {code} > public GraphTraversal< S , Edge > AddE (Direction direction, string > firstVertexKeyOrEdgeLabel, string edgeLabelOrSecondVertexKey, params object[] > propertyKeyValues); > public GraphTraversal< S , Edge > AddE (string edgeLabel); > {code} > Implementing this should make TINKERPOP-1725 obsolete and also resolve > TINKERPOP-1751. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[GitHub] tinkerpop pull request #712: TINKERPOP-1752: Gremlin.Net: Generate completel...
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/712#discussion_r141376043 --- Diff: gremlin-dotnet/src/Gremlin.Net/Process/Traversal/Bytecode.cs --- @@ -79,7 +85,77 @@ public void AddSource(string sourceName, params object[] args) /// The traversal method arguments. public void AddStep(string stepName, params object[] args) { -StepInstructions.Add(new Instruction(stepName, args)); +StepInstructions.Add(new Instruction(stepName, FlattenArguments(args))); +Bindings.Clear(); --- End diff -- That's a good point. So we should probably support lambdas. To be honest, I haven't really looked into what would be necessary to support lambdas. If I'm not mistaken then we can easily support Python and Groovy lambdas when we add a dedicated GraphSON serializer for them and probably a class that wraps the lambda together with its language. Then the steps that take a lambda would simply accept an object of that class in Gremlin.Net. I can create a separate ticket for this when we agree that we want to support them. When we want to support lambdas then we should also support `Bindings`. That leaves us with the question of whether we find a better implementation that doesn't suffer from the concurrency problems @jorgebay mentioned or if the current implementation in this pull request is acceptable. Apart from that, shouldn't we clarify the section in the documentation about bindings? It currently states for example [for gremlin-python](http://tinkerpop.apache.org/docs/current/reference/#_bindings): > If a traversal is going to be executed repeatedly, but with different parameters, then bindings should be used. because: > translation and compilation can be skipped as the previously compiled version should be cached which is only true when a lambda is used if I understood it correctly. ---
[jira] [Updated] (TINKERPOP-1792) Random TraversalSource Selection in GremlinScriptEngine
[ https://issues.apache.org/jira/browse/TINKERPOP-1792?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] stephen mallette updated TINKERPOP-1792: Component/s: python groovy > Random TraversalSource Selection in GremlinScriptEngine > --- > > Key: TINKERPOP-1792 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1792 > Project: TinkerPop > Issue Type: Bug > Components: groovy, python >Affects Versions: 3.2.6 >Reporter: stephen mallette >Priority: Critical > Labels: deprecation > > {{GremlinScriptEngine}} implementations make a random {{TraversalSource}} > selection when processing lambdas. Obviously it should be bound more clearly > to the specific {{TraversalSource}} the caller requests. Another issue that > isn't completely clear is that bindings passed from the client share the same > namespace as {{TraversalSource}} bindings which means that if the > {{TraversalSource}} is "g" then you couldn't write a traversal like: > {{withSideEffect(“g”, “hello”)}}. That could be smarter as well. -- This message was sent by Atlassian JIRA (v6.4.14#64029)