[GitHub] [accumulo] keith-turner commented on a diff in pull request #3400: fixes #3399: Access to configuration in context class loader factory

["keith-turner (via GitHub)" <gi...@apache.org>]

keith-turner commented on code in PR #3400:
URL: https://github.com/apache/accumulo/pull/3400#discussion_r1193862695


##########
core/src/main/java/org/apache/accumulo/core/spi/common/ContextClassLoaderFactory.java:
##########
@@ -57,4 +57,11 @@ public interface ContextClassLoaderFactory {
    */
   ClassLoader getClassLoader(String contextName);
 
+  /**
+   * Pass the service environment to allow for additional class loader configuration
+   *
+   * @param env the class loader environment
+   */
+  default void setEnvironment(ContextClassLoaderEnvironment env) {}

Review Comment:
   A lot of the spi interfaces have a common pattern of having an init method that a takes a single InitParameters interface like [here](https://github.com/apache/accumulo/blob/2f27dbd188ea5cee32f486599b97181b136fa787/core/src/main/java/org/apache/accumulo/core/spi/compaction/CompactionPlanner.java#L72) 
   
   ```suggestion
     default void init(InitParameters params) {}
   ```
   
   This could follow that pattern.  Also the init method name is good if its expected be called before anything else and once.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org