[jira] [Commented] (TINKERPOP-1793) addE() should allow dynamic edge labels

2017-09-27 Thread ASF GitHub Bot (JIRA) 

[ 
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...

2017-09-27 Thread robertdale 
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

2017-09-27 Thread ASF GitHub Bot (JIRA) 

[ 
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...

2017-09-27 Thread okram 
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

2017-09-27 Thread ASF GitHub Bot (JIRA) 

[ 
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...

2017-09-27 Thread dkuppitz 
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...

2017-09-27 Thread okram 
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

2017-09-27 Thread ASF GitHub Bot (JIRA) 

[ 
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

2017-09-27 Thread ASF GitHub Bot (JIRA) 

[ 
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...

2017-09-27 Thread FlorianHockmann 
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

2017-09-27 Thread stephen mallette (JIRA) 

 [ 
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)