dlmarion commented on code in PR #3678:
URL: https://github.com/apache/accumulo/pull/3678#discussion_r1287050648
##########
core/src/main/java/org/apache/accumulo/core/classloader/ClassLoaderUtil.java:
##########
@@ -82,6 +82,14 @@ public static ClassLoader getClassLoader(String context) {
}
}
+ public static boolean isValidContext(String context) {
+ if (context != null && !context.isEmpty()) {
Review Comment:
> The empty string is the default value for `table.class.loader.context`, so
it most definitely should be valid.
I'm not sure about that. There are no properties of PropertyType.STRING in
Property.java whose default value is `null`. They all either have a value, or
an empty string. I don't think that empty string is a valid String property
value in all cases. I'm sure that in some cases it means the property is not
set.
> If the context property is not set for a particular table, the factory
should still get consulted.
It's never worked like this. Even in `main` where the VFS stuff has been
ripped out ClassLoaderUtil only consults the ContextClassLoaderFactory when the
context name is set to a non-empty String. However, I think I have a solution -
I pushed the null and empty checks into the factory in a4750ab.
--
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]