keith-turner commented on code in PR #3678:
URL: https://github.com/apache/accumulo/pull/3678#discussion_r1288893673
##########
core/src/main/java/org/apache/accumulo/core/spi/common/ContextClassLoaderFactory.java:
##########
@@ -65,4 +65,15 @@ default void init(ContextClassLoaderEnvironment env) {}
* @return the class loader for the given contextName
*/
ClassLoader getClassLoader(String contextName);
+
+ /**
+ * Validate that the contextName is supported by the
ContextClassLoaderFactory implementation
+ *
+ * @param contextName the name of the context that represents a class loader
that is managed by
+ * this factory (can be null)
+ * @return true if valid, false otherwise
+ */
+ default boolean isValid(String contextName) {
+ return false;
+ }
Review Comment:
Had a discussion with @ctubbsii and @EdColeman about this PR. This new
methods default impl could call the existing getClassLoader() method. This
gives the opportunity to override if calling that for validation is
problematic, but does not require it.
```suggestion
default boolean isValid(String contextName) {
try{
var loader = getClassLoader(String contextName);
if(loader == null){
log.debug("Classloader context name {} is not valid, a null class
loader was returned", contextName);
return false;
}
return true;
} catch(RuntimeException e){
// TODO what log level should this be?
log.debug("Classloader context name {} is not valid", contextName,
e);
return false;
}
}
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
