keith-turner commented on code in PR #3683:
URL: https://github.com/apache/accumulo/pull/3683#discussion_r1290659206
##########
core/src/main/java/org/apache/accumulo/core/classloader/ClassLoaderUtil.java:
##########
@@ -75,22 +75,29 @@ static synchronized void resetContextFactoryForTests() {
@SuppressWarnings("deprecation")
public static ClassLoader getClassLoader(String context) {
- try {
- if (FACTORY.isValid(context)) {
- return FACTORY.getClassLoader(context);
- } else {
- return
org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader.getClassLoader();
- }
- } catch (RuntimeException e) {
+ if (context != null && !context.isEmpty()) {
Review Comment:
> This check was removed in https://github.com/apache/accumulo/pull/3678
based on @ctubbsii comments
https://github.com/apache/accumulo/pull/3678#discussion_r1285091831. We'll have
to reconcile the difference of opinion I think.
Thinking about that and this PR, there are two things I like about this PR.
* It follows a principal of least surprise in a bug fix release. It does
not change behavior of existing code in subtle ways and add new SPIs.
* There is no scope creep in the solution. Its a very targeted fix to the
problem of the context not being verified when its set as property.
--
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]
