[jira] [Commented] (TINKERPOP-1566) Kerberos authentication for gremlin-server

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1566?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15840424#comment-15840424
 ] 

ASF GitHub Bot commented on TINKERPOP-1566:
---

Github user vtslab commented on the issue:

https://github.com/apache/tinkerpop/pull/534
  
Hi @mike-tr-adamson, I am glad you entered the discussion. I think your 
main point is valid, namely that there are circumstances, pointed out by you, 
when gremlin-driver should select the GSSAPI mechanism even though no 
JAAS_ENTRY is specified (ToDo: make a test for this to safeguard the desired 
behavior).
Having said this, the old behavior (select GSSAPI out of the blue if no 
username/password is supplied) also has its risks and problems given the 
multitude of SASL mechanisms that people could want to use, see 
[http://www.iana.org/assignments/sasl-mechanisms/sasl-mechanisms.xhtml](http://www.iana.org/assignments/sasl-mechanisms/sasl-mechanisms.xhtml).
 Ideally, one would want gremlin-server to provide a token with the 
mechanism(s) it supports, so that gremlin-driver can use this to instantiate 
the SaslClient properly. 
In your case, with `javax.security.auth.useSubjectCredsOnly=false` 
configured, you would have a Gremlin-Server with a Krb5Authenticator 
configured, the server would provide the GSSAPI token in its authentication 
request and gremlin-driver would know to select the GSSAPI mechanism. 
However, this ideal situation requires more changes to the gremlin-driver 
and gremlin-server code. 
I could live now with adding the GSSException as an option to the tests 
with your explanation how it could be a valid option. This solves the current 
challenge and we can add this discussion as comments to the code for future 
reference, when requirements for other SASL mechanisms pop up.


> Kerberos authentication for gremlin-server
> --
>
> Key: TINKERPOP-1566
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1566
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: server
>Reporter: Marc de Lignie
>Priority: Minor
>  Labels: security
> Fix For: 3.3.0
>
>
> Gremlin server would benefit from an explicit Kerberos authentication plugin, 
> because preparing and maintaining such a plugin is nontrivial. Also, many 
> other Apache project provide kerberized services.
> In gremlin-console the standard Krb5LoginModule can be configured. 
> Gremlin-server already includes the pluggable Sasl framework that can host 
> the proposed Kerberos authentication plugin. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] tinkerpop issue #534: TINKERPOP-1566 Kerberos authentication for gremlin-ser...

[vtslab]

Github user vtslab commented on the issue:

https://github.com/apache/tinkerpop/pull/534
  
Hi @mike-tr-adamson, I am glad you entered the discussion. I think your 
main point is valid, namely that there are circumstances, pointed out by you, 
when gremlin-driver should select the GSSAPI mechanism even though no 
JAAS_ENTRY is specified (ToDo: make a test for this to safeguard the desired 
behavior).
Having said this, the old behavior (select GSSAPI out of the blue if no 
username/password is supplied) also has its risks and problems given the 
multitude of SASL mechanisms that people could want to use, see 
[http://www.iana.org/assignments/sasl-mechanisms/sasl-mechanisms.xhtml](http://www.iana.org/assignments/sasl-mechanisms/sasl-mechanisms.xhtml).
 Ideally, one would want gremlin-server to provide a token with the 
mechanism(s) it supports, so that gremlin-driver can use this to instantiate 
the SaslClient properly. 
In your case, with `javax.security.auth.useSubjectCredsOnly=false` 
configured, you would have a Gremlin-Server with a Krb5Authenticator 
configured, the server would provide the GSSAPI token in its authentication 
request and gremlin-driver would know to select the GSSAPI mechanism. 
However, this ideal situation requires more changes to the gremlin-driver 
and gremlin-server code. 
I could live now with adding the GSSException as an option to the tests 
with your explanation how it could be a valid option. This solves the current 
challenge and we can add this discussion as comments to the code for future 
reference, when requirements for other SASL mechanisms pop up.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (TINKERPOP-1617) Create a SingleIterationStrategy which will do its best to rewrite OLAP traversals to not message pass.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1617?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15840347#comment-15840347
 ] 

ASF GitHub Bot commented on TINKERPOP-1617:
---

GitHub user okram opened a pull request:

https://github.com/apache/tinkerpop/pull/549

TINKERPOP-1617: Create a SingleIterationStrategy which will do its best to 
rewrite OLAP traversals to not message pass.

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

There are various traversals that can be rewritten using `local()` that 
will enable the `GraphComputer` to avoid a message pass and thus, can 
accomplish the computation in a single scan of the graph. Benefiting traversal 
examples include:

```
g.V().out().id() --> g.V().local(out().id())
g.V().out().id().count() --> g.V().local(out().id()).count()
g.V().out().id().dedup().count()
g.V().inE().values("weight") // realize that in-edges are hosted by the 
out-vertex
g.V().inE().values("weight").sum()
g.V().both().count()
g.V().inE().count()
 g.V().as("a").outE().inV().as("b").id().dedup("a", "b").by(T.id).count()
```

Finally, the traversal that sparked this PR:

```
g.V().in().id().select("articleNumber").dedup().count()// requires one 
message pass

==translatesTo==>

g.V().local(in().id().select("articleNumber")).dedup().count() // requires 
no message passing
```

`SingleIterationStrategy` plays well with `SparkSingleIterationStrategy` 
which determines whether it is necessary to `cache()` and/or `partition()` the 
graph. If the traversal can be accomplished without a message pass (i.e. a 
single iteration), then performance is greatly improved as RDD partitions can 
be dropped as they are processed sequentially.

VOTE +1.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/apache/tinkerpop TINKERPOP-1617

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/tinkerpop/pull/549.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 #549






> Create a SingleIterationStrategy which will do its best to rewrite OLAP 
> traversals to not message pass.
> ---
>
> Key: TINKERPOP-1617
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1617
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: process
>Affects Versions: 3.2.3
>Reporter: Marko A. Rodriguez
>Assignee: Marko A. Rodriguez
>
> The traversal:
> {code}
> g.V().out().id().count()
> {code}
> Requires a message pass from {{out()}}. We shouldn't do this. Instead, if we 
> wrap the pre-barrier stage into a {{local()}}, we have:
> {code}
> g.V().local(out().id()).count()
> {code}
> ...which doesn't require a message pass and has the same semantics. This will 
> help open up numerous OLAP type traversals to single-pass/non-caching scans.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] tinkerpop pull request #549: TINKERPOP-1617: Create a SingleIterationStrateg...

[okram]

GitHub user okram opened a pull request:

https://github.com/apache/tinkerpop/pull/549

TINKERPOP-1617: Create a SingleIterationStrategy which will do its best to 
rewrite OLAP traversals to not message pass.

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

There are various traversals that can be rewritten using `local()` that 
will enable the `GraphComputer` to avoid a message pass and thus, can 
accomplish the computation in a single scan of the graph. Benefiting traversal 
examples include:

```
g.V().out().id() --> g.V().local(out().id())
g.V().out().id().count() --> g.V().local(out().id()).count()
g.V().out().id().dedup().count()
g.V().inE().values("weight") // realize that in-edges are hosted by the 
out-vertex
g.V().inE().values("weight").sum()
g.V().both().count()
g.V().inE().count()
 g.V().as("a").outE().inV().as("b").id().dedup("a", "b").by(T.id).count()
```

Finally, the traversal that sparked this PR:

```
g.V().in().id().select("articleNumber").dedup().count()// requires one 
message pass

==translatesTo==>

g.V().local(in().id().select("articleNumber")).dedup().count() // requires 
no message passing
```

`SingleIterationStrategy` plays well with `SparkSingleIterationStrategy` 
which determines whether it is necessary to `cache()` and/or `partition()` the 
graph. If the traversal can be accomplished without a message pass (i.e. a 
single iteration), then performance is greatly improved as RDD partitions can 
be dropped as they are processed sequentially.

VOTE +1.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/apache/tinkerpop TINKERPOP-1617

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/tinkerpop/pull/549.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 #549






---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Closed] (TINKERPOP-1433) Add steps to dev docs to help committers get their keys in order

[stephen mallette (JIRA)]

 [ 
https://issues.apache.org/jira/browse/TINKERPOP-1433?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

stephen mallette closed TINKERPOP-1433.
---
   Resolution: Fixed
Fix Version/s: (was: 3.3.0)

just quickly re-opened to give it the ticket one fix version. if you add more 
than one the release notes get sorta messed up. until we hit a point (which 
hopefully we won't) where code has diverted sufficiently such that a fix in a 
older line of code doesn't merge forward (or make sense) in a new line of code, 
it's easiest to set the fix version field to the lowest unreleased version that 
the ticket applies to.

> Add steps to dev docs to help committers get their keys in order
> 
>
> Key: TINKERPOP-1433
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1433
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: documentation
>Affects Versions: 3.1.4
>Reporter: Ted Wilmes
>Assignee: Ted Wilmes
> Fix For: 3.2.4
>
>
> I'll add some steps to help committers navigate the Apache PGP docs and will 
> include any TinkerPop specific steps (update validate-distribution.sh).



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Reopened] (TINKERPOP-1433) Add steps to dev docs to help committers get their keys in order

[stephen mallette (JIRA)]

 [ 
https://issues.apache.org/jira/browse/TINKERPOP-1433?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

stephen mallette reopened TINKERPOP-1433:
-

> Add steps to dev docs to help committers get their keys in order
> 
>
> Key: TINKERPOP-1433
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1433
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: documentation
>Affects Versions: 3.1.4
>Reporter: Ted Wilmes
>Assignee: Ted Wilmes
> Fix For: 3.3.0, 3.2.4
>
>
> I'll add some steps to help committers navigate the Apache PGP docs and will 
> include any TinkerPop specific steps (update validate-distribution.sh).



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Closed] (TINKERPOP-1433) Add steps to dev docs to help committers get their keys in order

[Ted Wilmes (JIRA)]

 [ 
https://issues.apache.org/jira/browse/TINKERPOP-1433?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Ted Wilmes closed TINKERPOP-1433.
-
   Resolution: Fixed
Fix Version/s: 3.2.4
   3.3.0

> Add steps to dev docs to help committers get their keys in order
> 
>
> Key: TINKERPOP-1433
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1433
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: documentation
>Affects Versions: 3.1.4
>Reporter: Ted Wilmes
>Assignee: Ted Wilmes
> Fix For: 3.3.0, 3.2.4
>
>
> I'll add some steps to help committers navigate the Apache PGP docs and will 
> include any TinkerPop specific steps (update validate-distribution.sh).



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Created] (TINKERPOP-1617) Create a SingleIterationStrategy which will do its best to rewrite OLAP traversals to not message pass.

[Marko A. Rodriguez (JIRA)]

Marko A. Rodriguez created TINKERPOP-1617:
-

 Summary: Create a SingleIterationStrategy which will do its best 
to rewrite OLAP traversals to not message pass.
 Key: TINKERPOP-1617
 URL: https://issues.apache.org/jira/browse/TINKERPOP-1617
 Project: TinkerPop
  Issue Type: Improvement
  Components: process
Affects Versions: 3.2.3
Reporter: Marko A. Rodriguez
Assignee: Marko A. Rodriguez


The traversal:

{code}
g.V().out().id().count()
{code}

Requires a message pass from {{out()}}. We shouldn't do this. Instead, if we 
wrap the pre-barrier stage into a {{local()}}, we have:

{code}
g.V().local(out().id()).count()
{code}

...which doesn't require a message pass and has the same semantics. This will 
help open up numerous OLAP type traversals to single-pass/non-caching scans.





--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Updated] (TINKERPOP-1616) Strengthen semantics around lazy iteration and graph modifications

[pieter martin (JIRA)]

 [ 
https://issues.apache.org/jira/browse/TINKERPOP-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

pieter martin updated TINKERPOP-1616:
-
Summary: Strengthen semantics around lazy iteration and graph modifications 
 (was: Strengthen semantics around lizy iteration and graph modifications)

> Strengthen semantics around lazy iteration and graph modifications
> --
>
> Key: TINKERPOP-1616
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1616
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: structure
>Affects Versions: 3.2.3
>Reporter: pieter martin
>
> The investigation started with the a bothE query where Sqlg returned 
> different results to TinkerGraph
> {code}
> @Test
> public void testLazy1() {
> final TinkerGraph graph = TinkerGraph.open();
> final Vertex a1 = graph.addVertex(T.label, "A");
> final Vertex b1 = graph.addVertex(T.label, "B");
> final Vertex c1 = graph.addVertex(T.label, "C");
> a1.addEdge("ab", b1);
> a1.addEdge("ac", c1);
> AtomicInteger count = new AtomicInteger(0);
> graph.traversal().V(a1).bothE().forEachRemaining(edge -> {
> a1.addEdge("ab", b1);
> c1.addEdge("ac", a1);
> count.getAndIncrement();
> });
> Assert.assertEquals(2, count.get());
> }
> {code}
> For this query TinkerGraph returns 2 and passes.
> Sqlg however returns 3. The reason being that it lazily iterates the out() 
> first and then the in().
> The following gremlin is the same but using a union(out(), in()) instead of 
> bothE()
> {code}
> @Test
> public void testLazy2() {
> final TinkerGraph graph = TinkerGraph.open();
> final Vertex a1 = graph.addVertex(T.label, "A");
> final Vertex b1 = graph.addVertex(T.label, "B");
> final Vertex c1 = graph.addVertex(T.label, "C");
> a1.addEdge("ab", b1);
> a1.addEdge("ac", c1);
> AtomicInteger count = new AtomicInteger(0);
> graph.traversal().V(a1).union(__.outE(), __.inE()).forEachRemaining(edge 
> -> {
> a1.addEdge("ab", b1);
> c1.addEdge("ac", a1);
> count.getAndIncrement();
> });
> Assert.assertEquals(2, count.get());
> }
> {code}
> In this case TinkerGraph returns 4 and Sqlg 6
> TinkerGraph returns 4 as it first walks the 2 out edges and adds 2 in edges 
> which it sees when traversing the in().
> Sqlg return 6 as it lazier than TinkerGraph.
> It first walks the "ac" out edge and adds in the 2 edges.
> Then walks "ab" and gets 2 edges. The original and the one added previously.
> It then walks "ac" in and gets 3 edges as 3 has been added so far.
> All and all 6.
> I am not sure whats the expected semantics. Sqlg is lazier than TinkerGraph 
> but not completely lazy either as it depends more on the meta data and number 
> of queries it needs to execute to walk a particular gremlin query.
> I am somewhat of the opinion that without enforcing a eager iteration when 
> modifying a graph the semantics will be different for different implementors.
> For Sqlg at least it will be hard for clients to predict the behavior.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Created] (TINKERPOP-1616) Strengthen semantics around lizy iteration and graph modifications

[pieter martin (JIRA)]

pieter martin created TINKERPOP-1616:


 Summary: Strengthen semantics around lizy iteration and graph 
modifications
 Key: TINKERPOP-1616
 URL: https://issues.apache.org/jira/browse/TINKERPOP-1616
 Project: TinkerPop
  Issue Type: Improvement
  Components: structure
Affects Versions: 3.2.3
Reporter: pieter martin


The investigation started with the a bothE query where Sqlg returned different 
results to TinkerGraph

{code}
@Test
public void testLazy1() {
final TinkerGraph graph = TinkerGraph.open();
final Vertex a1 = graph.addVertex(T.label, "A");
final Vertex b1 = graph.addVertex(T.label, "B");
final Vertex c1 = graph.addVertex(T.label, "C");
a1.addEdge("ab", b1);
a1.addEdge("ac", c1);

AtomicInteger count = new AtomicInteger(0);
graph.traversal().V(a1).bothE().forEachRemaining(edge -> {
a1.addEdge("ab", b1);
c1.addEdge("ac", a1);
count.getAndIncrement();
});
Assert.assertEquals(2, count.get());
}
{code}

For this query TinkerGraph returns 2 and passes.
Sqlg however returns 3. The reason being that it lazily iterates the out() 
first and then the in().

The following gremlin is the same but using a union(out(), in()) instead of 
bothE()

{code}
@Test
public void testLazy2() {
final TinkerGraph graph = TinkerGraph.open();
final Vertex a1 = graph.addVertex(T.label, "A");
final Vertex b1 = graph.addVertex(T.label, "B");
final Vertex c1 = graph.addVertex(T.label, "C");
a1.addEdge("ab", b1);
a1.addEdge("ac", c1);

AtomicInteger count = new AtomicInteger(0);
graph.traversal().V(a1).union(__.outE(), __.inE()).forEachRemaining(edge -> 
{
a1.addEdge("ab", b1);
c1.addEdge("ac", a1);
count.getAndIncrement();
});
Assert.assertEquals(2, count.get());
}
{code}

In this case TinkerGraph returns 4 and Sqlg 6

TinkerGraph returns 4 as it first walks the 2 out edges and adds 2 in edges 
which it sees when traversing the in().

Sqlg return 6 as it lazier than TinkerGraph.
It first walks the "ac" out edge and adds in the 2 edges.
Then walks "ab" and gets 2 edges. The original and the one added previously.
It then walks "ac" in and gets 3 edges as 3 has been added so far.
All and all 6.

I am not sure whats the expected semantics. Sqlg is lazier than TinkerGraph but 
not completely lazy either as it depends more on the meta data and number of 
queries it needs to execute to walk a particular gremlin query.

I am somewhat of the opinion that without enforcing a eager iteration when 
modifying a graph the semantics will be different for different implementors.

For Sqlg at least it will be hard for clients to predict the behavior.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Re: lazy iteration semantics

[Stephen Mallette]

I guess there is an inconsistency here that should be resolved.
Specifically, that means deciding on a behavior and then instituting
appropriate tests to enforce that behavior. I agree with Kuppitz that the
preferable way for this to work is to have that traversal be assert "2" as
that is the number of iterations through the traversal that occurred. I'm
not sure what the implementation challenges are with that, but that seems
most logical an approach to me. If that's the consensus here then I'd
suggest creating a ticket in JIRA and referencing this thread.

On Thu, Jan 26, 2017 at 11:14 AM, pieter-gmail 
wrote:

> Ping!
>
> Any ideas on this.
>
> Thanks
> Pieter
>
> On 23/01/2017 19:00, pieter-gmail wrote:
> > Hi,
> >
> > Ran some more tests, seems all is not well.
> >
> > So the previous example always works (on TinkerPop and Neo4j) because
> > bothE is not seen as two queries but rather one and all is well.
> >
> > Running the same test with a union instead has the same issue as Sqlg.
> >
> > @Test
> > public void testLazy() {
> > final TinkerGraph graph = TinkerGraph.open();
> > final Vertex a1 = graph.addVertex(T.label, "A");
> > final Vertex b1 = graph.addVertex(T.label, "B");
> > final Vertex c1 = graph.addVertex(T.label, "C");
> > a1.addEdge("ab", b1);
> > a1.addEdge("ac", c1);
> >
> > AtomicInteger count = new AtomicInteger(0);
> > graph.traversal().V(a1).union(outE(), inE()).forEachRemaining(edge
> -> {
> > a1.addEdge("ab", b1);#1
> > c1.addEdge("ac", a1);#2
> > count.getAndIncrement();
> > });
> > Assert.assertEquals(2, count.get());
> > }
> >
> > If only #1 is executed the test passes as the outE() is first traversed
> > and the subsequent inE() is unchanged.
> > However is #2 is executed by the time inE() executes its sees 2 new in
> > edges and the count == 4.
> >
> > I tested on Neo4j and it has the same behavior.
> >
> > As far as I can tell all is actually behaving as expected with lazy
> > iteration assumed.
> >
> > The unspecified semantics is with bothE(), both sides done immediately
> > like Neo4j and TinkerPop, or out and in done lazily as with Sqlg.
> > If lazily which side first as if affects the semantics.
> >
> > Another caveat is if barrier steps are injected and then the semantics
> > is not the same as without the barrier step.
> >
> > Cheers
> > Pieter
> >
> > On 23/01/2017 14:38, Stephen Mallette wrote:
> >> I'd say TinkerGraph demonstrates the expected behavior - I guess we
> don't
> >> have a test that enforces that?
> >>
> >> On Sat, Jan 21, 2017 at 3:23 PM, pieter-gmail 
> >> wrote:
> >>
> >>> Ok, thanks.
> >>>
> >>> Its tricky as it messes with the laziness and visibility of the
> queries.
> >>> Can really think of a solution though.
> >>>
> >>> Cheers
> >>> Pieter
> >>>
> >>> On 21/01/2017 21:14, Daniel Kuppitz wrote:
>  Hi Pieter,
> 
>  forEachRemaining iterates over the result and hence I would expect the
>  result to be 2. Otherwise you would / should end up with an endless
> loop.
>  However, the same behavior is seen when you replace forEachRemaining
> >>> with a
>  sideEffect lambda step. Not sure what the expected behavior is /
> should
> >>> be,
>  but I think I prefer the way it's done now.
> 
>  Cheers,
>  Daniel
> 
> 
>  On Sat, Jan 21, 2017 at 7:30 PM, pieter-gmail <
> pieter.mar...@gmail.com>
>  wrote:
> 
> > Hi,
> >
> > I need to clarify the expected semantics of gremlin's lazy iteration
> > semantics.
> > The following gremlin is what highlighted it.
> >
> > ```
> > @Test
> > public void testLazy() {
> > final TinkerGraph graph = TinkerGraph.open();
> > final Vertex a1 = graph.addVertex(T.label, "A");
> > final Vertex b1 = graph.addVertex(T.label, "B");
> > final Vertex c1 = graph.addVertex(T.label, "C");
> > a1.addEdge("ab", b1);
> > a1.addEdge("ac", c1);
> >
> > AtomicInteger count = new AtomicInteger(0);
> > graph.traversal().V(a1).bothE().forEachRemaining(edge -> {
> > a1.addEdge("ab", b1);
> > //a1.addEdge("ac", c1);
> > count.getAndIncrement();
> > });
> > Assert.assertEquals(2, count.get());
> > }
> >
> > ```
> >
> > On TinkerGraph the count is always 2.
> >
> > On Sqlg's Postgresql dialect the `bothE` first traverses the 'ac'
> edges
> > adds in a 'ab' edge and then traverses the 'ab' edges and ends up
> with a
> > count of 3.
> >
> > On Sqlg's HSQLDB and H2 dialects the `bothE` first traverses the 'ab'
> > edges adds in a 'ab' edge then traverses the 'ac' edges and ends up
> with
> > a count of 2.
> >
> > So for Sqlg the added edge will be seen by subsequent traversals due
> to
> > the 

Re: lazy iteration semantics

[pieter-gmail]

Ping!

Any ideas on this.

Thanks
Pieter

On 23/01/2017 19:00, pieter-gmail wrote:
> Hi,
>
> Ran some more tests, seems all is not well.
>
> So the previous example always works (on TinkerPop and Neo4j) because
> bothE is not seen as two queries but rather one and all is well.
>
> Running the same test with a union instead has the same issue as Sqlg.
>
> @Test
> public void testLazy() {
> final TinkerGraph graph = TinkerGraph.open();
> final Vertex a1 = graph.addVertex(T.label, "A");
> final Vertex b1 = graph.addVertex(T.label, "B");
> final Vertex c1 = graph.addVertex(T.label, "C");
> a1.addEdge("ab", b1);
> a1.addEdge("ac", c1);
>
> AtomicInteger count = new AtomicInteger(0);
> graph.traversal().V(a1).union(outE(), inE()).forEachRemaining(edge -> {
> a1.addEdge("ab", b1);#1
> c1.addEdge("ac", a1);#2
> count.getAndIncrement();
> });
> Assert.assertEquals(2, count.get());
> }
>
> If only #1 is executed the test passes as the outE() is first traversed
> and the subsequent inE() is unchanged.
> However is #2 is executed by the time inE() executes its sees 2 new in
> edges and the count == 4.
>
> I tested on Neo4j and it has the same behavior.
>
> As far as I can tell all is actually behaving as expected with lazy
> iteration assumed.
>
> The unspecified semantics is with bothE(), both sides done immediately
> like Neo4j and TinkerPop, or out and in done lazily as with Sqlg.
> If lazily which side first as if affects the semantics.
>
> Another caveat is if barrier steps are injected and then the semantics
> is not the same as without the barrier step.
>
> Cheers
> Pieter
>
> On 23/01/2017 14:38, Stephen Mallette wrote:
>> I'd say TinkerGraph demonstrates the expected behavior - I guess we don't
>> have a test that enforces that?
>>
>> On Sat, Jan 21, 2017 at 3:23 PM, pieter-gmail 
>> wrote:
>>
>>> Ok, thanks.
>>>
>>> Its tricky as it messes with the laziness and visibility of the queries.
>>> Can really think of a solution though.
>>>
>>> Cheers
>>> Pieter
>>>
>>> On 21/01/2017 21:14, Daniel Kuppitz wrote:
 Hi Pieter,

 forEachRemaining iterates over the result and hence I would expect the
 result to be 2. Otherwise you would / should end up with an endless loop.
 However, the same behavior is seen when you replace forEachRemaining
>>> with a
 sideEffect lambda step. Not sure what the expected behavior is / should
>>> be,
 but I think I prefer the way it's done now.

 Cheers,
 Daniel


 On Sat, Jan 21, 2017 at 7:30 PM, pieter-gmail 
 wrote:

> Hi,
>
> I need to clarify the expected semantics of gremlin's lazy iteration
> semantics.
> The following gremlin is what highlighted it.
>
> ```
> @Test
> public void testLazy() {
> final TinkerGraph graph = TinkerGraph.open();
> final Vertex a1 = graph.addVertex(T.label, "A");
> final Vertex b1 = graph.addVertex(T.label, "B");
> final Vertex c1 = graph.addVertex(T.label, "C");
> a1.addEdge("ab", b1);
> a1.addEdge("ac", c1);
>
> AtomicInteger count = new AtomicInteger(0);
> graph.traversal().V(a1).bothE().forEachRemaining(edge -> {
> a1.addEdge("ab", b1);
> //a1.addEdge("ac", c1);
> count.getAndIncrement();
> });
> Assert.assertEquals(2, count.get());
> }
>
> ```
>
> On TinkerGraph the count is always 2.
>
> On Sqlg's Postgresql dialect the `bothE` first traverses the 'ac' edges
> adds in a 'ab' edge and then traverses the 'ab' edges and ends up with a
> count of 3.
>
> On Sqlg's HSQLDB and H2 dialects the `bothE` first traverses the 'ab'
> edges adds in a 'ab' edge then traverses the 'ac' edges and ends up with
> a count of 2.
>
> So for Sqlg the added edge will be seen by subsequent traversals due to
> the lazy nature but the order affects the result.
>
> TinkerGraph seems to get both 'ab' and 'ac' edges upfront and does not
> subsequently see the added edge. A bit like it has a hidden barrier step
> before the `forEachRemaining`.
>
> What is the expected semantics in this situation?
> Is a traversal that traverses an element that has been modified earlier
> in the same traversal suppose to see the change?
>
> Thanks
> Pieter
>
>

RE: [DISCUSS] Release 3.2.4 and 3.1.6

[Paul A. Jackson]

OK, that's done, as you can no doubt see. Hoping this can make it into 3.2.4.

Thanks.

-Paul

-Original Message-
From: Stephen Mallette [mailto:spmalle...@gmail.com]
Sent: Wednesday, January 25, 2017 6:13 PM
To: dev@tinkerpop.apache.org
Subject: Re: [DISCUSS] Release 3.2.4 and 3.1.6

sure - please put that in the prefix of the PR title so it links back to that 
original jira ticket

On Wed, Jan 25, 2017 at 5:17 PM, Paul A. Jackson 
wrote:

> OK, low-risk PR coming for OLTP part. Should I reuse TINKERPOP-1589?
>
> Thanks,
> -Paul
>
> -Original Message-
> From: Stephen Mallette [mailto:spmalle...@gmail.com]
> Sent: Wednesday, January 25, 2017 5:07 PM
> To: dev@tinkerpop.apache.org
> Subject: Re: [DISCUSS] Release 3.2.4 and 3.1.6
>
> Being as close as we are to our code freeze/test week, I'd say that
> big, complex or otherwise risky changes probably won't collect too
> many +1 reviews at this point. If the OLTP improvement is
> small/concise (low risk), it could be considered for inclusion in 3.2.4.
>
> On Wed, Jan 25, 2017 at 5:00 PM, Paul A. Jackson 
> wrote:
>
> > I have something that fixes OLTP. I haven't worked with OLAP and it
> > looks like the changes for this will be extensive, touching
> > IteratorUtils and so on.
> >
> > Would you be interested in a PR for just the OLTP part?
> >
> >
> > -Original Message-
> > From: Stephen Mallette [mailto:spmalle...@gmail.com]
> > Sent: Wednesday, January 25, 2017 3:58 PM
> > To: dev@tinkerpop.apache.org
> > Subject: Re: [DISCUSS] Release 3.2.4 and 3.1.6
> >
> > in his case, it should go to tp32.
> >
> > On Wed, Jan 25, 2017 at 3:56 PM, Paul A. Jackson
> > 
> > wrote:
> >
> > > For what branch should a pull request be submitted?
> > >
> > >
> > > -Original Message-
> > > From: Stephen Mallette [mailto:spmalle...@gmail.com]
> > > Sent: Wednesday, January 25, 2017 3:41 PM
> > > To: dev@tinkerpop.apache.org
> > > Subject: Re: [DISCUSS] Release 3.2.4 and 3.1.6
> > >
> > > I went with the most obvious implementation place for
> CloseableIterator.
> > > If you see other spots where you could make an argument that it
> > > would make sense to add it then feel free to offer a pull request
> > > and we can get it reviewed. I didn't look into your VertexStep
> > > suggestion too deeply, but a quick review seems to have me
> > > thinking that it would make
> > sense to do that.
> > > Basically anywhere that a step interacts with the structure API
> > > seems like it would be a candidate for CloseableIterator to be
> > > implemented as it is in GraphStep.
> > >
> > > On Wed, Jan 25, 2017 at 3:30 PM, Paul A. Jackson
> > > 
> > > wrote:
> > >
> > > > So, I modified my code to work with CloseableIterator. I was
> > > > hoping this would be honored in more places than it is.
> > > >
> > > > Where it does work is if the user of a traversal calls
> > > > traversal.close() all the steps will get closed, including the
> > > > typically
> > > first GraphStep.
> > > > GraphStep in turn checks whether the iterator that was provided
> > > > by iteratorSupplier implements CloseableIterator and if so,
> > > > closes it, and this is good.
> > > >
> > > > What I was hoping, in addition, though, was when
> > > > VertexStep.flatMap() (or anything else) calls Vertex.vertices()
> > > > or
> > > > Vertex.edges() that before it finishes with the iterator it also
> > > > make the same check for CloseableIterator and call close().
> > > >
> > > > Thoughts?
> > > >
> > > > -Paul
> > > >
> > > > -Original Message-
> > > > From: Paul A. Jackson [mailto:paul.jack...@pb.com]
> > > > Sent: Tuesday, January 24, 2017 3:01 PM
> > > > To: dev@tinkerpop.apache.org
> > > > Subject: RE: [DISCUSS] Release 3.2.4 and 3.1.6
> > > >
> > > > Great. I'll try it out.
> > > >
> > > > -Paul
> > > >
> > > >
> > > > -Original Message-
> > > > From: Stephen Mallette [mailto:spmalle...@gmail.com]
> > > > Sent: Tuesday, January 24, 2017 2:54 PM
> > > > To: dev@tinkerpop.apache.org
> > > > Subject: Re: [DISCUSS] Release 3.2.4 and 3.1.6
> > > >
> > > > no - it's in 3.2.4 and merged forward to 3.3.0:
> > > >
> > > > https://github.com/apache/tinkerpop/blob/e3889bf2401b42c3afbc85e
> > > > ab
> > > > c2 fb c ebf2588974/gremlin-core/src/main/java/org/apache/
> > > > tinkerpop/gremlin/structure/util/CloseableIterator.java
> > > >
> > > > On Tue, Jan 24, 2017 at 2:52 PM, Paul A. Jackson
> > > > 
> > > > wrote:
> > > >
> > > > > Is CloseableIterator only in the 3.3 branch?
> > > > >
> > > > > -Paul
> > > > >
> > > > >
> > > > > -Original Message-
> > > > > From: Stephen Mallette [mailto:spmalle...@gmail.com]
> > > > > Sent: Monday, January 23, 2017 9:58 AM
> > > > > To: dev@tinkerpop.apache.org
> > > > > Subject: [DISCUSS] Release 3.2.4 and 3.1.6
> > > > >
> > > > > It's been a while since we've had a release (October 2016) and
> > > > > given the importance of the recent 

[jira] [Commented] (TINKERPOP-1583) PathRetractionStrategy retracts keys that are actually needed

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1583?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15839856#comment-15839856
 ] 

ASF GitHub Bot commented on TINKERPOP-1583:
---

Github user twilmes commented on the issue:

https://github.com/apache/tinkerpop/pull/546
  
It looks like when I run this against tp32 without 
`MatchPredicateStrategy`, the `a` and `b` keys are missing from the results.  I 
think I figured out what the issue is.  Without `MatchPredicateStrategy`, the 
wheres are not integrated into the `MatchStep` so `PathRetractionStrategy` sees 
that `a` and `b` are not referenced after the match so they are dropped.  I'll 
make an update so that if this pattern is seen, the match start and end labels 
will be preserved in the same manner as they would be with a traversal where 
the `MatchStep` was the terminating step. 


> PathRetractionStrategy retracts keys that are actually needed
> -
>
> Key: TINKERPOP-1583
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1583
> Project: TinkerPop
>  Issue Type: Bug
>  Components: process
>Affects Versions: 3.2.3
>Reporter: Geoff Reedy
>Assignee: Ted Wilmes
>
> We've seen this specifically for labels used in the until modulator of repeat 
> but I suspect it happens for other modulators as well. Here's a test case:
> {code}
> graph = TinkerGraph.open()
> g = graph.traversal()
> g.addV().as("first").repeat(addE("next").to(addV()).inV()).times(5).addE("next").to(select("first")).iterate()
> g.V().limit(1).as('z').out().repeat(store('seen').out().where(without('seen'))).until(where(eq('z')))
> {code}
> complains there is no z-key
> I tired to fix it myself and submit a pull request but I found the 
> implementation of PathRetractionStrategy confusing.
> One thing I noticed is that it seems the set of labels a step needs present 
> in order to work properly is determined external to the steps and that code 
> includes a lot of type-tests. If that logic were pushed down into the step 
> implementations I think fixing the repeat case would be easier and it would 
> be possible for extension steps to work properly with this strategy 
> (currently it seems they can't because of the closed-world assumption 
> inherent in the type-casing).



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] tinkerpop issue #546: TINKERPOP-1583: PathRetractionStrategy retracts keys t...

[twilmes]

Github user twilmes commented on the issue:

https://github.com/apache/tinkerpop/pull/546
  
It looks like when I run this against tp32 without 
`MatchPredicateStrategy`, the `a` and `b` keys are missing from the results.  I 
think I figured out what the issue is.  Without `MatchPredicateStrategy`, the 
wheres are not integrated into the `MatchStep` so `PathRetractionStrategy` sees 
that `a` and `b` are not referenced after the match so they are dropped.  I'll 
make an update so that if this pattern is seen, the match start and end labels 
will be preserved in the same manner as they would be with a traversal where 
the `MatchStep` was the terminating step. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (TINKERPOP-1589) Re-Introduce CloseableIterator

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1589?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15839825#comment-15839825
 ] 

ASF GitHub Bot commented on TINKERPOP-1589:
---

Github user pauljackson commented on the issue:

https://github.com/apache/tinkerpop/pull/548
  
Hadoop build is failing for me, but this happens under normal conditions.


> Re-Introduce CloseableIterator
> --
>
> Key: TINKERPOP-1589
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1589
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: structure
>Affects Versions: 3.2.3
>Reporter: stephen mallette
>Assignee: stephen mallette
> Fix For: 3.2.4
>
>
> In Blueprints, we used to have {{CloseableIterable}}, which allowed users to 
> release resources that might have been held by the underlying graph database. 
> We didn't port that interface to TinkerPop 3.x for some reason. Graphs like 
> OrientDB require a {{close()}} method for iterators returned from methods 
> like {{Graph.vertices()}} and {{Graph.edges()}}. In addition, allowing 
> {{close()}} from those iterators would make the {{close()}} on {{Traversal}} 
> (non-remote) more useful and meaningful (right now it is an empty method by 
> default). 
> In Blueprints, {{Graph.getVertices()}} and {{Graph.getEdges()}} returned a 
> {{Iterable}} and the Graph could choose to return {{CloseableIterator}}. 
> Users who wanted to write provider agnostic code would then have to do:
> {code}
> if (iterable instanceof CloseableIterable))
>   ((CloseableIterable) iterable).close();
> {code}
> I'm not sure why we didn't simply return {{CloseableIterable}} from those 
> methods. Any reason we shouldn't do that now? Perhaps the approach is to 
> introduce {{CloseableIterator}} to 3.2.4, but keep the return type of 
> {{edges/vertices()}} as {{Iterator}} and then for 3.3.0 change the return 
> type to {{CloseableIterator}}. In this way, the feature can be available in 
> next release of 3.2.4 without any API changes and users can do the 
> {{instanceof}} check above if they need to. Then for 3.3.0 when we allow API 
> changes we can just make it more convenient with a {{CloseableIterator}} 
> return type.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] tinkerpop issue #548: TINKERPOP-1589 Re-introduced CloseableIterator

[pauljackson]

Github user pauljackson commented on the issue:

https://github.com/apache/tinkerpop/pull/548
  
Hadoop build is failing for me, but this happens under normal conditions.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (TINKERPOP-1589) Re-Introduce CloseableIterator

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1589?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15839812#comment-15839812
 ] 

ASF GitHub Bot commented on TINKERPOP-1589:
---

GitHub user pauljackson opened a pull request:

https://github.com/apache/tinkerpop/pull/548

TINKERPOP-1589 Re-introduced CloseableIterator

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

Add support for the closing of `Iterators` returned from
`Vertex.vertices()` and `Vertex.edges()`.
Iterators are closed once they are read to completion.
Make `FlatMapStep` implement `AutoCloseable` in case iterator is not
read to completion, should get closed when Traversal is closed.
Remove unnecessary null check (null is never an instanceof).
OLTP mode support only. More extensive changes required for OLAP.
NOTE: Rethrowing checked Exception from `close()` as unchecked
RuntimeException in order to retain `Step.reset()` and
`AbstractStep.processNextStart()` signature.
Fix bug in last commit where iterator is not set to
EMPTY_ITERATOR for iterators that are not Closeable.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/pauljackson/tinkerpop tp32

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/tinkerpop/pull/548.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 #548


commit c8d5ff9db6ec8908abfad434c4fb339c5ad53468
Author: PaulJackson123 
Date:   2017-01-26T01:43:22Z

TINKERPOP-1589 Re-introduced CloseableIterator

https://issues.apache.org/jira/browse/TINKERPOP-1589
Add support for the closing of Iterators returned from
Vertex.vertices() and Vertex.edges().
Iterators are closed once they are read to completion.
Make FlatMapStep implement AutoCloseable in case iterator is not
read to completion, should get closed when Traversal is closed.
Remove unnecessary null check (null is never an instanceof).
OLTP mode support only. More extensive changes required for OLAP.
NOTE: Rethrowing checked Exception from close() as unchecked
RuntimeException in order to retain Step.reset() and
AbstractStep.processNextStart() signatured.

commit c2d183acf4418733666d1cd603f15a87beb9aa97
Author: PaulJackson123 
Date:   2017-01-26T14:52:30Z

TINKERPOP-1589 Re-introduced CloseableIterator

https://issues.apache.org/jira/browse/TINKERPOP-1589
Add support for the closing of Iterators returned from
Vertex.vertices() and Vertex.edges().
Iterators are closed once they are read to completion.
Make FlatMapStep implement AutoCloseable in case iterator is not
read to completion, should get closed when Traversal is closed.
Remove unnecessary null check (null is never an instanceof).
OLTP mode support only. More extensive changes required for OLAP.
NOTE: Rethrowing checked Exception from close() as unchecked
RuntimeException in order to retain Step.reset() and
AbstractStep.processNextStart() signatured.
Fix bug in last commit where iterator is not set to
EMPTY_ITERATOR for iterators that are not Closeable.




> Re-Introduce CloseableIterator
> --
>
> Key: TINKERPOP-1589
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1589
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: structure
>Affects Versions: 3.2.3
>Reporter: stephen mallette
>Assignee: stephen mallette
> Fix For: 3.2.4
>
>
> In Blueprints, we used to have {{CloseableIterable}}, which allowed users to 
> release resources that might have been held by the underlying graph database. 
> We didn't port that interface to TinkerPop 3.x for some reason. Graphs like 
> OrientDB require a {{close()}} method for iterators returned from methods 
> like {{Graph.vertices()}} and {{Graph.edges()}}. In addition, allowing 
> {{close()}} from those iterators would make the {{close()}} on {{Traversal}} 
> (non-remote) more useful and meaningful (right now it is an empty method by 
> default). 
> In Blueprints, {{Graph.getVertices()}} and {{Graph.getEdges()}} returned a 
> {{Iterable}} and the Graph could choose to return {{CloseableIterator}}. 
> Users who wanted to write provider agnostic code would then have to do:
> {code}
> if (iterable instanceof CloseableIterable))
>   ((CloseableIterable) iterable).close();
> {code}
> I'm not sure why we didn't simply return {{CloseableIterable}} from those 
> methods. Any reason we shouldn't do that now? Perhaps the approach is to 
> introduce {{CloseableIterator}} to 3.2.4, but keep the return type of 
> {{edges/vertices()}} as 

[GitHub] tinkerpop pull request #548: TINKERPOP-1589 Re-introduced CloseableIterator

[pauljackson]

GitHub user pauljackson opened a pull request:

https://github.com/apache/tinkerpop/pull/548

TINKERPOP-1589 Re-introduced CloseableIterator

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

Add support for the closing of `Iterators` returned from
`Vertex.vertices()` and `Vertex.edges()`.
Iterators are closed once they are read to completion.
Make `FlatMapStep` implement `AutoCloseable` in case iterator is not
read to completion, should get closed when Traversal is closed.
Remove unnecessary null check (null is never an instanceof).
OLTP mode support only. More extensive changes required for OLAP.
NOTE: Rethrowing checked Exception from `close()` as unchecked
RuntimeException in order to retain `Step.reset()` and
`AbstractStep.processNextStart()` signature.
Fix bug in last commit where iterator is not set to
EMPTY_ITERATOR for iterators that are not Closeable.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/pauljackson/tinkerpop tp32

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/tinkerpop/pull/548.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 #548


commit c8d5ff9db6ec8908abfad434c4fb339c5ad53468
Author: PaulJackson123 
Date:   2017-01-26T01:43:22Z

TINKERPOP-1589 Re-introduced CloseableIterator

https://issues.apache.org/jira/browse/TINKERPOP-1589
Add support for the closing of Iterators returned from
Vertex.vertices() and Vertex.edges().
Iterators are closed once they are read to completion.
Make FlatMapStep implement AutoCloseable in case iterator is not
read to completion, should get closed when Traversal is closed.
Remove unnecessary null check (null is never an instanceof).
OLTP mode support only. More extensive changes required for OLAP.
NOTE: Rethrowing checked Exception from close() as unchecked
RuntimeException in order to retain Step.reset() and
AbstractStep.processNextStart() signatured.

commit c2d183acf4418733666d1cd603f15a87beb9aa97
Author: PaulJackson123 
Date:   2017-01-26T14:52:30Z

TINKERPOP-1589 Re-introduced CloseableIterator

https://issues.apache.org/jira/browse/TINKERPOP-1589
Add support for the closing of Iterators returned from
Vertex.vertices() and Vertex.edges().
Iterators are closed once they are read to completion.
Make FlatMapStep implement AutoCloseable in case iterator is not
read to completion, should get closed when Traversal is closed.
Remove unnecessary null check (null is never an instanceof).
OLTP mode support only. More extensive changes required for OLAP.
NOTE: Rethrowing checked Exception from close() as unchecked
RuntimeException in order to retain Step.reset() and
AbstractStep.processNextStart() signatured.
Fix bug in last commit where iterator is not set to
EMPTY_ITERATOR for iterators that are not Closeable.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (TINKERPOP-1589) Re-Introduce CloseableIterator

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1589?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15839797#comment-15839797
 ] 

ASF GitHub Bot commented on TINKERPOP-1589:
---

Github user pauljackson closed the pull request at:

https://github.com/apache/tinkerpop/pull/547


> Re-Introduce CloseableIterator
> --
>
> Key: TINKERPOP-1589
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1589
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: structure
>Affects Versions: 3.2.3
>Reporter: stephen mallette
>Assignee: stephen mallette
> Fix For: 3.2.4
>
>
> In Blueprints, we used to have {{CloseableIterable}}, which allowed users to 
> release resources that might have been held by the underlying graph database. 
> We didn't port that interface to TinkerPop 3.x for some reason. Graphs like 
> OrientDB require a {{close()}} method for iterators returned from methods 
> like {{Graph.vertices()}} and {{Graph.edges()}}. In addition, allowing 
> {{close()}} from those iterators would make the {{close()}} on {{Traversal}} 
> (non-remote) more useful and meaningful (right now it is an empty method by 
> default). 
> In Blueprints, {{Graph.getVertices()}} and {{Graph.getEdges()}} returned a 
> {{Iterable}} and the Graph could choose to return {{CloseableIterator}}. 
> Users who wanted to write provider agnostic code would then have to do:
> {code}
> if (iterable instanceof CloseableIterable))
>   ((CloseableIterable) iterable).close();
> {code}
> I'm not sure why we didn't simply return {{CloseableIterable}} from those 
> methods. Any reason we shouldn't do that now? Perhaps the approach is to 
> introduce {{CloseableIterator}} to 3.2.4, but keep the return type of 
> {{edges/vertices()}} as {{Iterator}} and then for 3.3.0 change the return 
> type to {{CloseableIterator}}. In this way, the feature can be available in 
> next release of 3.2.4 without any API changes and users can do the 
> {{instanceof}} check above if they need to. Then for 3.3.0 when we allow API 
> changes we can just make it more convenient with a {{CloseableIterator}} 
> return type.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (TINKERPOP-1589) Re-Introduce CloseableIterator

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1589?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15839796#comment-15839796
 ] 

ASF GitHub Bot commented on TINKERPOP-1589:
---

Github user pauljackson commented on the issue:

https://github.com/apache/tinkerpop/pull/547
  
Resubmitting this pull request.


> Re-Introduce CloseableIterator
> --
>
> Key: TINKERPOP-1589
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1589
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: structure
>Affects Versions: 3.2.3
>Reporter: stephen mallette
>Assignee: stephen mallette
> Fix For: 3.2.4
>
>
> In Blueprints, we used to have {{CloseableIterable}}, which allowed users to 
> release resources that might have been held by the underlying graph database. 
> We didn't port that interface to TinkerPop 3.x for some reason. Graphs like 
> OrientDB require a {{close()}} method for iterators returned from methods 
> like {{Graph.vertices()}} and {{Graph.edges()}}. In addition, allowing 
> {{close()}} from those iterators would make the {{close()}} on {{Traversal}} 
> (non-remote) more useful and meaningful (right now it is an empty method by 
> default). 
> In Blueprints, {{Graph.getVertices()}} and {{Graph.getEdges()}} returned a 
> {{Iterable}} and the Graph could choose to return {{CloseableIterator}}. 
> Users who wanted to write provider agnostic code would then have to do:
> {code}
> if (iterable instanceof CloseableIterable))
>   ((CloseableIterable) iterable).close();
> {code}
> I'm not sure why we didn't simply return {{CloseableIterable}} from those 
> methods. Any reason we shouldn't do that now? Perhaps the approach is to 
> introduce {{CloseableIterator}} to 3.2.4, but keep the return type of 
> {{edges/vertices()}} as {{Iterator}} and then for 3.3.0 change the return 
> type to {{CloseableIterator}}. In this way, the feature can be available in 
> next release of 3.2.4 without any API changes and users can do the 
> {{instanceof}} check above if they need to. Then for 3.3.0 when we allow API 
> changes we can just make it more convenient with a {{CloseableIterator}} 
> return type.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] tinkerpop pull request #547: TINKERPOP-1589 Re-introduced CloseableIterator

[pauljackson]

Github user pauljackson closed the pull request at:

https://github.com/apache/tinkerpop/pull/547


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tinkerpop issue #547: TINKERPOP-1589 Re-introduced CloseableIterator

[pauljackson]

Github user pauljackson commented on the issue:

https://github.com/apache/tinkerpop/pull/547
  
Resubmitting this pull request.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] tinkerpop issue #534: TINKERPOP-1566 Kerberos authentication for gremlin-ser...

[mike-tr-adamson]

Github user mike-tr-adamson commented on the issue:

https://github.com/apache/tinkerpop/pull/534
  
I'm concerned about changing the `getMechanism` logic based on the failing 
tests. The tests can legitimately fail both ways. It would be valid to fix 
those tests so that they can fail on either a `ResponseException` or a 
`GSSException`. As has already been said the difference in error being whether 
the test finds any kerberos configuration that it can use.

The tests were failing on my machine for exactly the same reason - I had an 
`/etc/krb5.conf` file.

I'm not that hung up on the change because you have to provide a 
`JAAS_ENTRY` in order to use kerberos. It's more of a principal thing. The 
tests were failing on my machine prior to this PR so the failure was not 
related to any of these changes. As such they could be fixed outside of this PR 
in which case the `getMechanism` change wouldn't be needed. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: [DISCUSS] Release 3.2.4 and 3.1.6

[Ted Wilmes]

I've only attempted running it from the docker build thus far.  I'm
thinking it has
to be some odd local environment issue.  I'll try again here in a bit and
see if I
can nail down the root cause.

On Thu, Jan 26, 2017 at 8:29 AM, Stephen Mallette 
wrote:

> Thanks Ted. Does the GLV issue have something to do with your python setup?
> or is it a problem with the docs themselves?
>
> On Thu, Jan 26, 2017 at 9:26 AM, Ted Wilmes  wrote:
>
> > Jason,
> > I ctr'ed an update to docs adding instructions for getting keys setup.
> For
> > some reason
> > I'm getting consistent failures in the GLV portion of the docs when I'm
> > generation them so I haven't
> > published updated snapshots yet.  For the time being though, if you
> haven't
> > started the key process
> > yet, you can see the info here under Release Manager Requirements:
> > https://github.com/apache/tinkerpop/blob/master/docs/
> > src/dev/developer/release.asciidoc
> >
> > --Ted
> >
> > On Wed, Jan 25, 2017 at 5:13 PM, Stephen Mallette 
> > wrote:
> >
> > > sure - please put that in the prefix of the PR title so it links back
> to
> > > that original jira ticket
> > >
> > > On Wed, Jan 25, 2017 at 5:17 PM, Paul A. Jackson 
> > > wrote:
> > >
> > > > OK, low-risk PR coming for OLTP part. Should I reuse TINKERPOP-1589?
> > > >
> > > > Thanks,
> > > > -Paul
> > > >
> > > > -Original Message-
> > > > From: Stephen Mallette [mailto:spmalle...@gmail.com]
> > > > Sent: Wednesday, January 25, 2017 5:07 PM
> > > > To: dev@tinkerpop.apache.org
> > > > Subject: Re: [DISCUSS] Release 3.2.4 and 3.1.6
> > > >
> > > > Being as close as we are to our code freeze/test week, I'd say that
> > big,
> > > > complex or otherwise risky changes probably won't collect too many +1
> > > > reviews at this point. If the OLTP improvement is small/concise (low
> > > risk),
> > > > it could be considered for inclusion in 3.2.4.
> > > >
> > > > On Wed, Jan 25, 2017 at 5:00 PM, Paul A. Jackson <
> paul.jack...@pb.com>
> > > > wrote:
> > > >
> > > > > I have something that fixes OLTP. I haven't worked with OLAP and it
> > > > > looks like the changes for this will be extensive, touching
> > > > > IteratorUtils and so on.
> > > > >
> > > > > Would you be interested in a PR for just the OLTP part?
> > > > >
> > > > >
> > > > > -Original Message-
> > > > > From: Stephen Mallette [mailto:spmalle...@gmail.com]
> > > > > Sent: Wednesday, January 25, 2017 3:58 PM
> > > > > To: dev@tinkerpop.apache.org
> > > > > Subject: Re: [DISCUSS] Release 3.2.4 and 3.1.6
> > > > >
> > > > > in his case, it should go to tp32.
> > > > >
> > > > > On Wed, Jan 25, 2017 at 3:56 PM, Paul A. Jackson <
> > paul.jack...@pb.com>
> > > > > wrote:
> > > > >
> > > > > > For what branch should a pull request be submitted?
> > > > > >
> > > > > >
> > > > > > -Original Message-
> > > > > > From: Stephen Mallette [mailto:spmalle...@gmail.com]
> > > > > > Sent: Wednesday, January 25, 2017 3:41 PM
> > > > > > To: dev@tinkerpop.apache.org
> > > > > > Subject: Re: [DISCUSS] Release 3.2.4 and 3.1.6
> > > > > >
> > > > > > I went with the most obvious implementation place for
> > > > CloseableIterator.
> > > > > > If you see other spots where you could make an argument that it
> > > > > > would make sense to add it then feel free to offer a pull request
> > > > > > and we can get it reviewed. I didn't look into your VertexStep
> > > > > > suggestion too deeply, but a quick review seems to have me
> thinking
> > > > > > that it would make
> > > > > sense to do that.
> > > > > > Basically anywhere that a step interacts with the structure API
> > > > > > seems like it would be a candidate for CloseableIterator to be
> > > > > > implemented as it is in GraphStep.
> > > > > >
> > > > > > On Wed, Jan 25, 2017 at 3:30 PM, Paul A. Jackson
> > > > > > 
> > > > > > wrote:
> > > > > >
> > > > > > > So, I modified my code to work with CloseableIterator. I was
> > > > > > > hoping this would be honored in more places than it is.
> > > > > > >
> > > > > > > Where it does work is if the user of a traversal calls
> > > > > > > traversal.close() all the steps will get closed, including the
> > > > > > > typically
> > > > > > first GraphStep.
> > > > > > > GraphStep in turn checks whether the iterator that was provided
> > by
> > > > > > > iteratorSupplier implements CloseableIterator and if so, closes
> > > > > > > it, and this is good.
> > > > > > >
> > > > > > > What I was hoping, in addition, though, was when
> > > > > > > VertexStep.flatMap() (or anything else) calls Vertex.vertices()
> > or
> > > > > > > Vertex.edges() that before it finishes with the iterator it
> also
> > > > > > > make the same check for CloseableIterator and call close().
> > > > > > >
> > > > > > > Thoughts?
> > > > > > >
> > > > > > > -Paul
> > > > > > >
> > > > > > > -Original Message-
> > > > > > > From: Paul A. Jackson 

Re: [DISCUSS] Release 3.2.4 and 3.1.6

[Stephen Mallette]

Thanks Ted. Does the GLV issue have something to do with your python setup?
or is it a problem with the docs themselves?

On Thu, Jan 26, 2017 at 9:26 AM, Ted Wilmes  wrote:

> Jason,
> I ctr'ed an update to docs adding instructions for getting keys setup.  For
> some reason
> I'm getting consistent failures in the GLV portion of the docs when I'm
> generation them so I haven't
> published updated snapshots yet.  For the time being though, if you haven't
> started the key process
> yet, you can see the info here under Release Manager Requirements:
> https://github.com/apache/tinkerpop/blob/master/docs/
> src/dev/developer/release.asciidoc
>
> --Ted
>
> On Wed, Jan 25, 2017 at 5:13 PM, Stephen Mallette 
> wrote:
>
> > sure - please put that in the prefix of the PR title so it links back to
> > that original jira ticket
> >
> > On Wed, Jan 25, 2017 at 5:17 PM, Paul A. Jackson 
> > wrote:
> >
> > > OK, low-risk PR coming for OLTP part. Should I reuse TINKERPOP-1589?
> > >
> > > Thanks,
> > > -Paul
> > >
> > > -Original Message-
> > > From: Stephen Mallette [mailto:spmalle...@gmail.com]
> > > Sent: Wednesday, January 25, 2017 5:07 PM
> > > To: dev@tinkerpop.apache.org
> > > Subject: Re: [DISCUSS] Release 3.2.4 and 3.1.6
> > >
> > > Being as close as we are to our code freeze/test week, I'd say that
> big,
> > > complex or otherwise risky changes probably won't collect too many +1
> > > reviews at this point. If the OLTP improvement is small/concise (low
> > risk),
> > > it could be considered for inclusion in 3.2.4.
> > >
> > > On Wed, Jan 25, 2017 at 5:00 PM, Paul A. Jackson 
> > > wrote:
> > >
> > > > I have something that fixes OLTP. I haven't worked with OLAP and it
> > > > looks like the changes for this will be extensive, touching
> > > > IteratorUtils and so on.
> > > >
> > > > Would you be interested in a PR for just the OLTP part?
> > > >
> > > >
> > > > -Original Message-
> > > > From: Stephen Mallette [mailto:spmalle...@gmail.com]
> > > > Sent: Wednesday, January 25, 2017 3:58 PM
> > > > To: dev@tinkerpop.apache.org
> > > > Subject: Re: [DISCUSS] Release 3.2.4 and 3.1.6
> > > >
> > > > in his case, it should go to tp32.
> > > >
> > > > On Wed, Jan 25, 2017 at 3:56 PM, Paul A. Jackson <
> paul.jack...@pb.com>
> > > > wrote:
> > > >
> > > > > For what branch should a pull request be submitted?
> > > > >
> > > > >
> > > > > -Original Message-
> > > > > From: Stephen Mallette [mailto:spmalle...@gmail.com]
> > > > > Sent: Wednesday, January 25, 2017 3:41 PM
> > > > > To: dev@tinkerpop.apache.org
> > > > > Subject: Re: [DISCUSS] Release 3.2.4 and 3.1.6
> > > > >
> > > > > I went with the most obvious implementation place for
> > > CloseableIterator.
> > > > > If you see other spots where you could make an argument that it
> > > > > would make sense to add it then feel free to offer a pull request
> > > > > and we can get it reviewed. I didn't look into your VertexStep
> > > > > suggestion too deeply, but a quick review seems to have me thinking
> > > > > that it would make
> > > > sense to do that.
> > > > > Basically anywhere that a step interacts with the structure API
> > > > > seems like it would be a candidate for CloseableIterator to be
> > > > > implemented as it is in GraphStep.
> > > > >
> > > > > On Wed, Jan 25, 2017 at 3:30 PM, Paul A. Jackson
> > > > > 
> > > > > wrote:
> > > > >
> > > > > > So, I modified my code to work with CloseableIterator. I was
> > > > > > hoping this would be honored in more places than it is.
> > > > > >
> > > > > > Where it does work is if the user of a traversal calls
> > > > > > traversal.close() all the steps will get closed, including the
> > > > > > typically
> > > > > first GraphStep.
> > > > > > GraphStep in turn checks whether the iterator that was provided
> by
> > > > > > iteratorSupplier implements CloseableIterator and if so, closes
> > > > > > it, and this is good.
> > > > > >
> > > > > > What I was hoping, in addition, though, was when
> > > > > > VertexStep.flatMap() (or anything else) calls Vertex.vertices()
> or
> > > > > > Vertex.edges() that before it finishes with the iterator it also
> > > > > > make the same check for CloseableIterator and call close().
> > > > > >
> > > > > > Thoughts?
> > > > > >
> > > > > > -Paul
> > > > > >
> > > > > > -Original Message-
> > > > > > From: Paul A. Jackson [mailto:paul.jack...@pb.com]
> > > > > > Sent: Tuesday, January 24, 2017 3:01 PM
> > > > > > To: dev@tinkerpop.apache.org
> > > > > > Subject: RE: [DISCUSS] Release 3.2.4 and 3.1.6
> > > > > >
> > > > > > Great. I'll try it out.
> > > > > >
> > > > > > -Paul
> > > > > >
> > > > > >
> > > > > > -Original Message-
> > > > > > From: Stephen Mallette [mailto:spmalle...@gmail.com]
> > > > > > Sent: Tuesday, January 24, 2017 2:54 PM
> > > > > > To: dev@tinkerpop.apache.org
> > > > > > Subject: Re: [DISCUSS] Release 3.2.4 

Re: [DISCUSS] Release 3.2.4 and 3.1.6

[Ted Wilmes]

Jason,
I ctr'ed an update to docs adding instructions for getting keys setup.  For
some reason
I'm getting consistent failures in the GLV portion of the docs when I'm
generation them so I haven't
published updated snapshots yet.  For the time being though, if you haven't
started the key process
yet, you can see the info here under Release Manager Requirements:
https://github.com/apache/tinkerpop/blob/master/docs/src/dev/developer/release.asciidoc

--Ted

On Wed, Jan 25, 2017 at 5:13 PM, Stephen Mallette 
wrote:

> sure - please put that in the prefix of the PR title so it links back to
> that original jira ticket
>
> On Wed, Jan 25, 2017 at 5:17 PM, Paul A. Jackson 
> wrote:
>
> > OK, low-risk PR coming for OLTP part. Should I reuse TINKERPOP-1589?
> >
> > Thanks,
> > -Paul
> >
> > -Original Message-
> > From: Stephen Mallette [mailto:spmalle...@gmail.com]
> > Sent: Wednesday, January 25, 2017 5:07 PM
> > To: dev@tinkerpop.apache.org
> > Subject: Re: [DISCUSS] Release 3.2.4 and 3.1.6
> >
> > Being as close as we are to our code freeze/test week, I'd say that big,
> > complex or otherwise risky changes probably won't collect too many +1
> > reviews at this point. If the OLTP improvement is small/concise (low
> risk),
> > it could be considered for inclusion in 3.2.4.
> >
> > On Wed, Jan 25, 2017 at 5:00 PM, Paul A. Jackson 
> > wrote:
> >
> > > I have something that fixes OLTP. I haven't worked with OLAP and it
> > > looks like the changes for this will be extensive, touching
> > > IteratorUtils and so on.
> > >
> > > Would you be interested in a PR for just the OLTP part?
> > >
> > >
> > > -Original Message-
> > > From: Stephen Mallette [mailto:spmalle...@gmail.com]
> > > Sent: Wednesday, January 25, 2017 3:58 PM
> > > To: dev@tinkerpop.apache.org
> > > Subject: Re: [DISCUSS] Release 3.2.4 and 3.1.6
> > >
> > > in his case, it should go to tp32.
> > >
> > > On Wed, Jan 25, 2017 at 3:56 PM, Paul A. Jackson 
> > > wrote:
> > >
> > > > For what branch should a pull request be submitted?
> > > >
> > > >
> > > > -Original Message-
> > > > From: Stephen Mallette [mailto:spmalle...@gmail.com]
> > > > Sent: Wednesday, January 25, 2017 3:41 PM
> > > > To: dev@tinkerpop.apache.org
> > > > Subject: Re: [DISCUSS] Release 3.2.4 and 3.1.6
> > > >
> > > > I went with the most obvious implementation place for
> > CloseableIterator.
> > > > If you see other spots where you could make an argument that it
> > > > would make sense to add it then feel free to offer a pull request
> > > > and we can get it reviewed. I didn't look into your VertexStep
> > > > suggestion too deeply, but a quick review seems to have me thinking
> > > > that it would make
> > > sense to do that.
> > > > Basically anywhere that a step interacts with the structure API
> > > > seems like it would be a candidate for CloseableIterator to be
> > > > implemented as it is in GraphStep.
> > > >
> > > > On Wed, Jan 25, 2017 at 3:30 PM, Paul A. Jackson
> > > > 
> > > > wrote:
> > > >
> > > > > So, I modified my code to work with CloseableIterator. I was
> > > > > hoping this would be honored in more places than it is.
> > > > >
> > > > > Where it does work is if the user of a traversal calls
> > > > > traversal.close() all the steps will get closed, including the
> > > > > typically
> > > > first GraphStep.
> > > > > GraphStep in turn checks whether the iterator that was provided by
> > > > > iteratorSupplier implements CloseableIterator and if so, closes
> > > > > it, and this is good.
> > > > >
> > > > > What I was hoping, in addition, though, was when
> > > > > VertexStep.flatMap() (or anything else) calls Vertex.vertices() or
> > > > > Vertex.edges() that before it finishes with the iterator it also
> > > > > make the same check for CloseableIterator and call close().
> > > > >
> > > > > Thoughts?
> > > > >
> > > > > -Paul
> > > > >
> > > > > -Original Message-
> > > > > From: Paul A. Jackson [mailto:paul.jack...@pb.com]
> > > > > Sent: Tuesday, January 24, 2017 3:01 PM
> > > > > To: dev@tinkerpop.apache.org
> > > > > Subject: RE: [DISCUSS] Release 3.2.4 and 3.1.6
> > > > >
> > > > > Great. I'll try it out.
> > > > >
> > > > > -Paul
> > > > >
> > > > >
> > > > > -Original Message-
> > > > > From: Stephen Mallette [mailto:spmalle...@gmail.com]
> > > > > Sent: Tuesday, January 24, 2017 2:54 PM
> > > > > To: dev@tinkerpop.apache.org
> > > > > Subject: Re: [DISCUSS] Release 3.2.4 and 3.1.6
> > > > >
> > > > > no - it's in 3.2.4 and merged forward to 3.3.0:
> > > > >
> > > > > https://github.com/apache/tinkerpop/blob/e3889bf2401b42c3afbc85eab
> > > > > c2 fb c ebf2588974/gremlin-core/src/main/java/org/apache/
> > > > > tinkerpop/gremlin/structure/util/CloseableIterator.java
> > > > >
> > > > > On Tue, Jan 24, 2017 at 2:52 PM, Paul A. Jackson
> > > > > 
> > > > > wrote:
> > > > >
> > > > > > Is