ivakegg commented on code in PR #3400:
URL: https://github.com/apache/accumulo/pull/3400#discussion_r1202518087
##########
core/src/main/java/org/apache/accumulo/core/classloader/DefaultContextClassLoaderFactory.java:
##########
Review Comment:
ok
##########
core/src/main/java/org/apache/accumulo/core/classloader/ClassLoaderUtil.java:
##########
@@ -36,25 +39,30 @@ private ClassLoaderUtil() {
/**
* Initialize the ContextClassLoaderFactory
*/
- public static synchronized void initContextFactory(AccumuloConfiguration
conf) {
+ public static synchronized void initContextFactory(final
AccumuloConfiguration conf) {
if (FACTORY == null) {
LOG.debug("Creating {}", ContextClassLoaderFactory.class.getName());
String factoryName =
conf.get(Property.GENERAL_CONTEXT_CLASSLOADER_FACTORY);
if (factoryName == null || factoryName.isEmpty()) {
- // load the default implementation
LOG.info("Using default {}, which is subject to change in a future
release",
ContextClassLoaderFactory.class.getName());
- FACTORY = new DefaultContextClassLoaderFactory(conf);
- } else {
- // load user's selected implementation
- try {
- var factoryClass =
Class.forName(factoryName).asSubclass(ContextClassLoaderFactory.class);
- LOG.info("Creating {}: {}",
ContextClassLoaderFactory.class.getName(), factoryName);
- FACTORY = factoryClass.getDeclaredConstructor().newInstance();
- } catch (ReflectiveOperationException e) {
- throw new IllegalStateException("Unable to load and initialize
class: " + factoryName, e);
- }
+ factoryName = DefaultContextClassLoaderFactory.class.getName();
Review Comment:
Adding a check to ensure DefaultContextClassLoaderFactory is not set as the
property name. IMHO I think this is not reasonable. If you are going to
supply a default implementation, then let that be part of the external API.
--
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]
