ctubbsii commented on pull request #1596: URL: https://github.com/apache/accumulo/pull/1596#issuecomment-619299148
> This method will load all the iterators configured in the provided properties and scope. Placed method in TableOperations since it seemed similar to other iterator methods and uses table properties but is really just a static utility method. What is the clear and overwhelming value to end users for having this in the public API? I ask because this seems more useful for internal use and maybe for iterator testing (which we already have a separate iterator framework for that). I'm not necessarily opposed... but adding new stuff to the API adds a long-term obligation for the project. Iterators, in particular, are at higher risk of being negatively impacted by prior API decisions (see previous attempts to mark them closeable, or to add client-side iterators for offline scanning). So, I want to make sure I fully understand the value and that we've properly weighed the consequences before adding it as new public API. > This implementation always uses the Java classloader by calling `.useAccumuloClassLoader(false);`. I wasn't sure if we want to provide an option for using the AccumuloVFSClassLoader. Depends on the intent here. At the very least, I would not want to pass a boolean. If it is added, I would make the API accept an instance of `java.lang.ClassLoader`. This would help us support future work to loosen the tight coupling the custom ClassLoader stuff has with Accumulo bootup in `accumulo-start.jar`. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
