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 [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to