paul-rogers opened a new pull request, #12816:
URL: https://github.com/apache/druid/pull/12816

   See [Issue #12815](https://github.com/apache/druid/issues/12815) for an 
overview of the goal of this PR. This PR provides the basic refactoring to 
allow a later PR to restructure the Calcite test setup which will in turn 
simply the proposed Druid planner test framework. This PR also lays the 
groundwork to simplify the complex, ad-hoc code which the "new ITs" use to set 
up tests.
   
   ### Refactors Guice Injector Construction
   
   See the above issue for an overview.
   
   #### `ExtensionsLoader`
   
   The `Initialization` class currently performa a kitchen-sink set of tasks, 
one of which is to load extensions. That code was pulled out into a separate 
class: `ExtensionsLoader`. The previous code used global maps to hold extension 
information, and cobbled together a way to pass a config object into static 
`Initialization` methods.
   
   The new approach is that `ExtensionsLoader` is Guice-enabled: it is created 
by the startup injector and passed to the primary injector. Guice, then, 
manages the singleton for this class. Clients of the code now obtain an 
`ExtensionsLoader` from Guice.
   
   `InitializationTest` cases related to extensions moved into a new 
``ExtensionsLoaderTest` class.
   
   ### Hadoop Initialization
   
   The `Initialization` class also holds a method, 
`getHadoopDependencyFilesToLoad()`, used only by Hadoop indexing tasks. This 
code moved to a new `Initialization` class in the indexing module. The test 
cases moved to a new test class.
   
   ### `Initialization` Deprecation
   
   The `Initialization` class is now virtually empty:
   
   * Hadoop-related code moved as described above.
   * Extension-related code moved as described above.
   * Module management moved into injector builders as described in [Issue 
#12815](https://github.com/apache/druid/issues/12815).
   
   The only functionality left n `Initialization` is `makeServerInjector()` 
called from `GuiceRunable` and a test case. There is also the original 
`makeInjectorWithModules()`, now marked as deprecated, to build an injector. 
However, this method adds all the server dependencies, which is not what is 
wanted for tests. Over time, tests will be refactored to build an injector with 
just the modules needed, and without server baggage. At that time, we'll drop 
the `makeInjectorWithModules()` method. The remaining method, 
`makeServerInjector()`, can move into `GuiceRunable`.
   
   ### `GuiceInjectors` Deprecation
   
   Similarly, the `GuiceInjectors` class builds the startup injector. As tests 
shift to use the builders, `GuiceInjectors` can disappear.
   
   <hr>
   
   This PR has:
   - [X] been self-reviewed.
   - [X] added Javadocs for most classes and all non-trivial methods. Linked 
related entities via Javadoc links.
   - [X] added comments explaining the "why" and the intent of the code 
wherever would not be obvious for an unfamiliar reader.
   - [X] added unit tests or modified existing tests to cover new code paths, 
ensuring the threshold for [code 
coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md)
 is met.
   - [ ] been tested in a test Druid cluster.
   


-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to