ctubbsii commented on code in PR #3678:
URL: https://github.com/apache/accumulo/pull/3678#discussion_r1286681871
##########
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:
If the context property is not set for a particular table, the factory
should still get consulted. The empty string is the default value for
`table.class.loader.context`, so it most definitely should be valid. The
factory should get passed an empty string if the user hasn't overridden it with
anything else. It should be up to the factory to handle that according to its
own implementation... whether it passes through to the system classloader, or
contains a built-in default classloader to return is up to the implementation.
As for the part of the code you point to, I'm not exactly sure what it's
doing there... I would think that it should be possible to override a value of
`"default"` with a value of `""`, but that code you pointed to seemed to
interpret `""` as `unset()`. I'm not sure it's doing the right thing... but
that would be a separate issue. Right now, I'm just concerned about what values
the factory should allow... and I don't think we need to constrain it to only
see non-empty String values as valid, when an empty String is the default, and
should be perfectly valid. I'm also of the opinion that `null` should be valid
as well. For example, if we had a context classloader factory implementation
called `AlwaysReturnSystemClassLoader`, then all context values are valid, and
would result in returning the system class loader, with the actual value
ignored. That trivial implementation seems like it should be allowed, but it
won't work if constraints about validity are imposed outside the SPI imp
lementation.
--
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]
