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

["ASF GitHub Bot (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/TINKERPOP-1589?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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)