Add ExtensionsConfig.excludeModules#4438
Conversation
| "Extension module [%s] was ignored because it doesn't have a canonical name, is it a local or anonymous class?", | ||
| module.getClass().getName() | ||
| ); | ||
| } else if (config.getExcludeModules().contains(moduleName)) { |
There was a problem hiding this comment.
would it make sense to extend this functionality with an ability to exclude modules on the package level (or by regex) instead of explicitly enumerate all of them by class name?
There was a problem hiding this comment.
I don't want to do this for a number of reasons:
- First of all, we don't need it
- It's more complex and raises small issues on implementation level, e. g. for regex you need to recognize package dots which are not regex dots
- List of class names is the type of property which is already used in Druid, don't want to add more types like regexes, glob patterns, etc.
- If somebody want to exclude many modules from some extension, likely it's a better idea to extract those modules into a separate extension and simply not add this new extension to the loadList.
There was a problem hiding this comment.
Keeping the same behavior as loadList is best, IMO.
| |`druid.extensions.directory`|The root extension directory where user can put extensions related files. Druid will load extensions stored under this directory.|`extensions` (This is a relative path to Druid's working directory)| | ||
| |`druid.extensions.hadoopDependenciesDir`|The root hadoop dependencies directory where user can put hadoop related dependencies files. Druid will load the dependencies based on the hadoop coordinate specified in the hadoop index task.|`hadoop-dependencies` (This is a relative path to Druid's working directory| | ||
| |`druid.extensions.loadList`|A JSON array of extensions to load from extension directories by Druid. If it is not specified, its value will be `null` and Druid will load all the extensions under `druid.extensions.directory`. If its value is empty list `[]`, then no extensions will be loaded at all. It is also allowed to specify absolute path of other custom extensions not stored in the common extensions directory.|null| | ||
| |`druid.extensions.excludeModules`|A JSON array of canonical class names (e. g. `"io.druid.somepackage.SomeClass"`) of modules, which shouldn't be loaded, despite they are found in the `loadList`. Useful when some useful extension contains some module, which shouldn't be loaded on some Druid node type because of some dependencies of that module couldn't be satisfied.|[]| |
There was a problem hiding this comment.
druid.extensions.excludeList for symmetry with loadList?
There was a problem hiding this comment.
Direct symmetry between loadList and excludeList could be confusing because loadList is a list of extension names, while excludeList is a list of class names. Renamed to moduleExcludeList.
There was a problem hiding this comment.
Ah, I see. In that case, could you please change the wording of of this part:
modules, which shouldn't be loaded, despite they are found in the
loadList
It makes it sound like the things in moduleExcludeList are the same kind of things as found in loadList. That's what confused me. Perhaps wording like this would be clearer.
A JSON array of canonical class names (e. g.
"io.druid.somepackage.SomeClass") of module classes which shouldn't be loaded, even if they are found in extensions specified bydruid.extensions.loadList.
There was a problem hiding this comment.
@gianm changed.
I also changed return types of getFromExtensions() and getLoadedModules() and renamed getLoadedModules() to getLoadedImplementations(), assuming Initialization is not a public API.
* Add ExtensionsConfig.excludeModules * Add branch * Refactor Initialization.getFromExtensions() * excludeModules -> moduleExcludeList * Initialization.getFromExtensions() and getLoadedModules() should return Collection, not Set * Fix doc
4 participants
Add ability to exclude module classes from extensions.