dlmarion commented on code in PR #3683:
URL: https://github.com/apache/accumulo/pull/3683#discussion_r1291244666
##########
core/src/test/java/org/apache/accumulo/core/classloader/URLClassLoaderFactory.java:
##########
@@ -54,19 +54,4 @@ public ClassLoader getClassLoader(String contextName) {
throw new IllegalArgumentException("Error creating URL from contextName:
" + contextName, e);
}
}
-
Review Comment:
There were other changes in URLClassLoaderFactory as part of 3678 that could
be backed out, might be easier to revert.
##########
core/src/main/java/org/apache/accumulo/core/classloader/DefaultContextClassLoaderFactory.java:
##########
@@ -102,20 +101,4 @@ public ClassLoader getClassLoader(String contextName) {
return org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader
.getContextClassLoader(contextName);
}
-
Review Comment:
With your change in ClassLoaderUtil above, which essentially reverts the
changes in #3678, I think you can remove the contextName checks in
getClassLoader (lines 98-100). They were added in #3678. I wonder if it would
be easier to revert #3678 and then apply the changes in ShellServerIT,
PropUtil, SystemPropUtil, and ClassLoaderUtil.
##########
start/src/main/java/org/apache/accumulo/start/classloader/vfs/ContextManager.java:
##########
@@ -158,14 +158,6 @@ public synchronized void setContextConfig(ContextsConfig
config) {
this.config = config;
}
- public boolean isKnownContext(String contextName) {
Review Comment:
The `public` modifier was added to the class in #3678 , that could be backed
out as well.
--
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]
