ctubbsii commented on code in PR #3136:
URL: https://github.com/apache/accumulo/pull/3136#discussion_r1052580972
##########
core/src/main/java/org/apache/accumulo/core/conf/Property.java:
##########
@@ -1092,21 +1079,8 @@ public enum Property {
+ "constraint.",
"2.0.0"),
- // VFS ClassLoader properties
-
- // this property shouldn't be used directly; it exists solely to document
the default value
- // defined by its use in AccumuloVFSClassLoader when generating the property
documentation
- @Deprecated(since = "2.1.0", forRemoval = true)
- VFS_CLASSLOADER_SYSTEM_CLASSPATH_PROPERTY(
-
org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader.VFS_CLASSLOADER_SYSTEM_CLASSPATH_PROPERTY,
- "", PropertyType.STRING,
- "Configuration for a system level vfs classloader. Accumulo jar can be"
- + " configured here and loaded out of HDFS.",
- "1.5.0"),
- @Deprecated(since = "2.1.0", forRemoval = true)
- VFS_CONTEXT_CLASSPATH_PROPERTY(
-
org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader.VFS_CONTEXT_CLASSPATH_PROPERTY,
- null, PropertyType.PREFIX,
+ // ClassLoader properties
+ CONTEXT_CLASSPATH_PROPERTY(ContextManager.CONTEXT_CLASSPATH_PROPERTY, null,
PropertyType.PREFIX,
Review Comment:
The idea previously discussed was that the
`general.context.class.loader.factory` is the singleton service for creating
class loaders based on a specified context in the `table.class.loader.context`
property. This is also what the descriptions of those properties say. So, the
interpretation of the `table.class.loader.context` value is dependent on which
implementation factory has been set in `general.context.class.loader.factory`.
If you set a URLClassLoader factory, you can just set the
`table.class.loader.context` property to be a set of URLs. You don't need a
separate property at all. You can also have a more advanced factory that just
takes a simplified name, and maps that name to a set of URLs or whatever, in
its own configuration file.
Basically, all we need is the factory that does `String context ->
ClassLoader` mapping, and the property that stores the `String context`. That
context could be a name, a number, a JSON blob, a comma-separated set of URLs
(or using another delimiter than a comma), or whatever the user can imagine for
their particular implementation.
By default, though, we discussed having a URLClassLoader that interprets the
context field as a set of URLs. If you're concerned about misconfiguration, the
implementation could require the set of URLs have some kind of prefix to
validate, as in `urls:url1,url2,url3,...`, and a separate project could create
a VFS classloader that had similar validation, `vfs:url1,url2,url3,...`, so
mis-configurations are more detectable. But, validation like that is not a
requirement of the implementing factory.
--
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]
