adutra commented on code in PR #3340:
URL: https://github.com/apache/polaris/pull/3340#discussion_r2681823617
##########
runtime/server/src/main/java/org/apache/polaris/admintool/BootstrapCommand.java:
##########
Review Comment:
I don't think keeping the admin module around is "over-splitting".
This PR, while interesting in that it achieves a single artifact/jar for
both usages (server and CLI), does create an imbalanced layout where the top
module includes code for the CLI, but the code for the server lives in a
dependency:
```mermaid
flowchart TD
A["`polaris-server<br/>(incl. admin tool)`"] --> B
B[polaris-runtime-service]
```
Whereas @dimas-b proposal still keeps the single entrypoint but each usage
has its own module:
```mermaid
flowchart TD
A[polaris-server] --> B
A --> C
B[polaris-runtime-service] --> D
C[polaris-admin] --> E
B --> F
C --> F
D[specific deps]
E[specific deps]
F[common deps]
```
This has a net advantage imho of leaving `polaris-server` as a mere
"assembly descriptor" as @dimas-b puts it.
> It makes the dependency graph harder to reason about, and forces consumers
and tests to pull in modules that exist only to host a thin slice of logic
I don't buy much this argument: while a proliferation of modules can
complicate the dependency graph, modularity offers many upsides: modern build
tools efficiently manage even hundreds of dependencies; a DAG enforces clearer
mental models about dependencies than monoliths that often create dependency
cycles; smaller modules accelerate compilation; and downstream integrators
benefit from leaner CDI contexts.
Polaris is currently in an intermediate, non-optimal state: it's neither a
monolith, nor a well organized modular application. We should rethink the
purpose of those "common" modules, but also consider separating disparate
concerns in modules like `polaris-runtime-service`: what does telemetry have to
do with authentication? And what does the secrets manager have to do with
events processing? I support moving towards a well-thought modular
organization, and I think @dimas-b suggestion goes in that direction.
--
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]
