Remove Settings.getAsClass #12744
Remove Settings.getAsClass #12744
Conversation
|
That's fantastic! |
clintongormley
added
>bug
blocker
review
:Core/Infra/Core
|
+1 |
| */ | ||
| public interface SpawnModules { | ||
| @Deprecated | ||
| public interface SpawnShittyModules { |
There was a problem hiding this comment.
sorry... -1 revert the name
There was a problem hiding this comment.
yeah - no need to rename this
|
I left some comments other than that LGTM. I'd really like to work on making all our internally plugged in mock things a |
|
LGTM |
|
this is a great step forward. Let's get rid of spawn modules altogether in a followup we need to flatten this hierarchy... |
|
oh and please squash these - it's pretty unreadable list of commits :) |
the default classloader. It had all kinds of leniency in how the classname was found, and simply cannot work with plugins having isolated classloaders. This change removes that method. Some of the uses of it were for custom extension points, like custom repository or discovery types. A lot were just there to plugin mock implementations for tests. For the settings that were legitimate, all now support plugins adding the given setting via onModule. For those that were specific to tests for mocks, they now use Classes.loadClass (a helper around Class.forName). This is a temporary measure until (in a future PR) tests can change the implementation via package private statics. I also removed a number of unnecessary intermediate modules, added a "jvm-example" plugin that can be filled in in the future as a smoke test for breaking plugins, and gave some documentation to "spawn" modules interface. closes elastic#12643 closes elastic#12656
|
I created #12783 as a follow up. |
|
I also created #12784 to finish the test refactoring work here. |
This method on settings loaded a class, based on a setting value, using the default classloader. It had all kinds of leniency in how the classname was found, and simply cannot work with plugins having isolated classloaders.
This change removes that method. Some of the uses of it were for custom extension points, like custom repository or discovery types. A lot were just there to plugin mock implementations for tests. For the settings that were legitimate, all now support plugins adding the given setting via onModule. For those that were specific to tests for mocks, they now use Classes.loadClass (a helper around Class.forName). This is a temporary measure until (in a future PR) tests can change the implementation via package private statics.
I also removed a number of unnecessary intermediate modules, added a "jvm-example" plugin that can be filled in in the future as a smoke test for breaking plugins, and gave some documentation to "spawn" modules interface.
closes #12643
closes #12656