ctubbsii commented on code in PR #3400:
URL: https://github.com/apache/accumulo/pull/3400#discussion_r1194151189
##########
core/src/main/java/org/apache/accumulo/core/spi/common/ContextClassLoaderFactory.java:
##########
@@ -57,4 +57,11 @@ public interface ContextClassLoaderFactory {
*/
ClassLoader getClassLoader(String contextName);
+ /**
+ * Pass the service environment to allow for additional class loader
configuration
+ *
+ * @param env the class loader environment
+ */
+ default void setEnvironment(ContextClassLoaderEnvironment env) {}
Review Comment:
I don't mind the name change for the method, but the `InitParameters`
doesn't seem to be as accurate as the `...Environment` name. `InitParameters`
implies to me that there are fixed params at initialization. I think that's a
bit misleading. What's actually being passed is essentially
`Supplier<ServiceEnvironment.Configuration>`, which isn't a fixed set of
parameters, but a way to peek into the current system configuration, as needed.
If it's called `InitParameters`, and the method is still
`getConfiguration()`, then that seems to imply a fixed configuration at
initialization, when really the only thing that's fixed is the mechanism to get
the configuration contained within the environment, not the configuration
itself. So, if it's called `InitParameters`, I think the method would need to
be changed to something like `.environmentConfigurationSupplier()`, so it's
clear that the *supplier* is the fixed parameter, and the configuration is part
of the environmental state, not just the state at initialization.
In putting together my suggested changes in ivakegg/accumulo#5, I considered
suggesting getting rid of the ContextClassLoaderEnvironment entirely, and
replace it with a `Supplier<ServiceEnvironment.Configuration>`, but that seemed
too limiting, in case we want to expose more of the environment. Ideally, we'd
pass the entire `ServiceEnvironment`, instead of just the configuration, but I
explained why that was a problem in ivakegg/accumulo#5.
--
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]
