[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=15850003#comment-15850003
 ] 

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

Github user asfgit closed the pull request at:

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


> 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.15#6346)

[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=15849966#comment-15849966
 ] 

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

Github user dkuppitz commented on the issue:

https://github.com/apache/tinkerpop/pull/548
  
VOTE: +1


> 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.15#6346)

[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=15849797#comment-15849797
 ] 

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

Github user spmallette commented on the issue:

https://github.com/apache/tinkerpop/pull/548
  
All tests pass with `docker/build.sh -t -n -i`

VOTE +1


> 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.15#6346)

[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=15848886#comment-15848886
 ] 

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

Github user okram commented on the issue:

https://github.com/apache/tinkerpop/pull/548
  
VOTE +1.


> 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.15#6346)

[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=15848819#comment-15848819
 ] 

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

Github user pauljackson commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/548#discussion_r98973022
  
--- Diff: 
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/FlatMapStep.java
 ---
@@ -44,6 +45,7 @@ public FlatMapStep(final Traversal.Admin traversal) {
 return this.head.split(this.iterator.next(), this);
 } else {
 this.head = this.starts.next();
+closeIterator();
--- End diff --

I'll update the PR.


> 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.15#6346)

[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=15848817#comment-15848817
 ] 

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

Github user pauljackson commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/548#discussion_r98972948
  
--- Diff: 
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java
 ---
@@ -164,13 +164,13 @@ public int hashCode() {
 }
 
 /**
- * Attemps to close an underlying iterator if it is of type {@link 
CloseableIterator}. Graph providers may choose
+ * Attempts to close an underlying iterator if it is of type {@link 
CloseableIterator}. Graph providers may choose
  * to return this interface containing their vertices and edges if 
there are expensive resources that might need to
  * be released at some point.
  */
 @Override
 public void close() throws Exception {
-if (iterator != null && iterator instanceof CloseableIterator) {
--- End diff --

`GraphStep` doesn't extend `FlatMapStep`, so it doesn't have access to 
`closeIterator()`, but it can call `CloseableIterator.closeIterator(iterator)`


> 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.15#6346)

[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=15848573#comment-15848573
 ] 

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

Github user okram commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/548#discussion_r98930854
  
--- Diff: 
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/FlatMapStep.java
 ---
@@ -44,6 +45,7 @@ public FlatMapStep(final Traversal.Admin traversal) {
 return this.head.split(this.iterator.next(), this);
 } else {
 this.head = this.starts.next();
+closeIterator();
--- End diff --

This should go before `this.starts.next()` as if `this.starts.next()` is 
empty, it will throw `FastNoSuchElementException` and the previous iterator 
would not have been closed.


> 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.15#6346)

[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=15848574#comment-15848574
 ] 

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

Github user okram commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/548#discussion_r98931140
  
--- Diff: 
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java
 ---
@@ -164,13 +164,13 @@ public int hashCode() {
 }
 
 /**
- * Attemps to close an underlying iterator if it is of type {@link 
CloseableIterator}. Graph providers may choose
+ * Attempts to close an underlying iterator if it is of type {@link 
CloseableIterator}. Graph providers may choose
  * to return this interface containing their vertices and edges if 
there are expensive resources that might need to
  * be released at some point.
  */
 @Override
 public void close() throws Exception {
-if (iterator != null && iterator instanceof CloseableIterator) {
--- End diff --

Why doesn't `GraphStep` implement `AutoCloseable` like the others?


> 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.15#6346)

[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=15843288#comment-15843288
 ] 

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

Github user spmallette commented on the issue:

https://github.com/apache/tinkerpop/pull/548
  
I'm pretty sure @okram was affirming that `VertexStep`, `EdgeVertexStep` 
and `PropertiesStep` should implement `AutoCloseable` (I don't think that there 
are others) rather than a blanket change of just applying it to `FlatMapStep`. 
As the logic is re-used, perhaps you could do an `ElementStep` that extends the 
`FlatMapStep` and implements `AutoCloseable`. Then have those three steps 
extend `ElementStep` which would contain the close logic that you have.


> 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=15843138#comment-15843138
 ] 

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

Github user spmallette commented on the issue:

https://github.com/apache/tinkerpop/pull/548
  
ha - "makes sense" that the way it is now is ok or "makes sense" as with 
the change i proposed? :smile:


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

[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 

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

[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=15839015#comment-15839015
 ] 

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

GitHub user pauljackson opened a pull request:

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

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()` signatures.

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/547.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 #547


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.




> 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=15811793#comment-15811793
 ] 

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

Github user asfgit closed the pull request at:

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


> 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=15801995#comment-15801995
 ] 

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

Github user twilmes commented on the issue:

https://github.com/apache/tinkerpop/pull/521
  
VOTE: +1


> 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=15801625#comment-15801625
 ] 

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

Github user lvca commented on the issue:

https://github.com/apache/tinkerpop/pull/521
  
VOTE +1


> 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=15801597#comment-15801597
 ] 

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

Github user okram commented on the issue:

https://github.com/apache/tinkerpop/pull/521
  
VOTE +1


> 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=15801586#comment-15801586
 ] 

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

Github user spmallette commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/521#discussion_r94782450
  
--- Diff: 
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java
 ---
@@ -254,9 +254,14 @@ public default void forEachRemaining(final Consumer action) {
 }
 }
 
+/**
+ * Releases resources opened in any steps that implement {@link 
AutoCloseable}.
+ */
 @Override
 public default void close() throws Exception {
--- End diff --

changes are made now - thanks - didn't know that stuff.


> 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=15801494#comment-15801494
 ] 

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

Github user okram commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/521#discussion_r94776952
  
--- Diff: 
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java
 ---
@@ -254,9 +254,14 @@ public default void forEachRemaining(final Consumer action) {
 }
 }
 
+/**
+ * Releases resources opened in any steps that implement {@link 
AutoCloseable}.
+ */
 @Override
 public default void close() throws Exception {
--- End diff --

This is bad. `Steps` that are `TraversalParents` are responsible for 
propagating operations on them to those below. I would do the following:

```
for(final Step step : this.getSteps()) {
  if(step instanceof AutoCloseable)
step.close();
}
```

Next, I would have `TraversalParent`  extend `AutoCloseable` with its 
default `close()` method being:

```
for(final Traversal.Admin traversal : this.getLocalChildren()) {
  traversal.close()
}

for(final Traversal.Admin 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=15801463#comment-15801463
 ] 

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

Github user spmallette commented on the issue:

https://github.com/apache/tinkerpop/pull/521
  
I think I'm done - It's already implemented on `Traversal.close()`. I think 
it's good for vote. Don't think i'm missing anything.


> 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=15801415#comment-15801415
 ] 

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

Github user okram commented on the issue:

https://github.com/apache/tinkerpop/pull/521
  
Nice. Clean backwards compatible `GraphStep.close()` method which uses 
`instanceof`.

Are you looking for a VOTE now or are you still building on this PR? -- 
e.g. integrating it with `Traversal.close()`/etc.


> 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=15801081#comment-15801081
 ] 

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

GitHub user spmallette opened a pull request:

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

TINKERPOP-1589 Re-introduced CloseableIterator

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

Made it so that the `CloseableIterator` is closed by `GraphStep` if it is 
provided by the iterator supplier. Furthermore, any steps in a `Traversal` 
implement `AutoCloseable` then its `close()` method will be called. In this way 
`Traversal.close()`, the common API a user will work with, has a way to release 
resources in `Graph` implementations that require it.

Builds with `docker/build.sh -t - i -n`.

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

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

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

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


commit 3597dc5f06407a8b1ada0e49a52b36b2f31b8a34
Author: Stephen Mallette 
Date:   2017-01-04T21:14:23Z

TINKERPOP-1589 Re-introduced CloseableIterator

Made it so that the CloseableIterator is closed by GraphStep if it is 
provided by the iterator supplier. Furthermore, any steps in a Traversal 
implement AutoCloseable then its close() method will be called.




> 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

[stephen mallette (JIRA)]

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

stephen mallette commented on TINKERPOP-1589:
-

Those suggestions sound reasonable. 

I think I will go back on some of the stuff I said above in the description. I 
don't think we need to change the {{vertices()}} and {{edges()}} API for 3.3.0 
anymore. The return types can probably just remain as {{Iterator}}. I don't 
think there is much user confusion to avoid here as we don't encourage users 
operating at the Graph Structure API level. Everything is done through the 
Traversal API. Therefore, they wouldn't be calling those methods on {{Graph}} 
in the first place. 



> 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

[Paul Jackson (JIRA)]

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

Paul Jackson commented on TINKERPOP-1589:
-

This all sounds good.

If you chose to make those methods return CloseableIterator, you could make it 
slightly easier for providers by giving CloseableIterator:
1) A default close() that does nothing, and
2) A static CloseableIterator asCloseableIterator(Iterator iterator) 
method that wraps the supplied Iterator with a default implementation (if the 
supplied iterator wasn't instanceof CloseableIterator).

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