accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From milleruntime <...@git.apache.org>
Subject [GitHub] accumulo pull request #159: ACCUMULO-1280: many changes for closing iterator...
Date Fri, 18 Nov 2016 13:00:32 GMT
Github user milleruntime commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/159#discussion_r88652597
  
    --- Diff: core/src/main/java/org/apache/accumulo/core/iterators/SortedKeyValueIterator.java
---
    @@ -147,4 +147,13 @@
        *              if not supported.
        */
       SortedKeyValueIterator<K,V> deepCopy(IteratorEnvironment env);
    +
    +  /**
    +   * Closes the Iterator. This should be overridden by the implementing class that has
access to <tt>SortedKeyValueIterator source</tt> provided in the
    --- End diff --
    
    Thanks for the link but I think a "SHOULD" is the most appropriate. We don't want to say
MUST since there may not be anything to close. 
    
    I think the ambiguity stems from the flexibility we have with the Iterators.  Look at
the doc for init for instance: "Initializes the iterator. Data should not be read from the
source in this method."  Do we want to make them more rigid?
    
    I don't mention Exceptions because I wasn't sure what to say. Sometimes we eat, log and
continue and sometimes we throw.  Do we have best practices for handling exceptions?
    
    I can change the "Typically" part to be a little more specific. 


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message