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

Reply via email to