Plugin manager rewrite as plugin installers#16082
Plugin manager rewrite as plugin installers#16082skrzypo987 wants to merge 1 commit intotrinodb:masterfrom
Conversation
skrzypo987
changed the title
skrzypo987
force-pushed
the
skrzypo/266-plugin-manager-rewrite-as-plugin-installer
branch
from
25e2180 to
c4532af
Compare
There was a problem hiding this comment.
does this method has to be public? Can you please make it private? Do you need @VisibleForTests? Same comment for all other classes
There was a problem hiding this comment.
Reduced the visibility when applicable.
There was a problem hiding this comment.
please define the logger as private static final. Same for other places like that.
There was a problem hiding this comment.
It's an interface so just Logger log = Logger.get(ResourceGroupManager.class);.
Changed
core/trino-main/src/main/java/io/trino/exchange/ExchangeManagerRegistry.java
Outdated
Show resolved
Hide resolved
core/trino-main/src/main/java/io/trino/server/ServerMainModule.java
Outdated
Show resolved
Hide resolved
core/trino-main/src/main/java/io/trino/testing/LocalQueryRunner.java
Outdated
Show resolved
Hide resolved
skrzypo987
force-pushed
the
skrzypo/266-plugin-manager-rewrite-as-plugin-installer
branch
from
c4532af to
5a50036
Compare
|
@kokosing AC |
|
Please rebase |
ksobolew
left a comment
ksobolew
left a comment
There was a problem hiding this comment.
Just wanted to say that I like it
introduced in the previous commit. The exception being connector factories since they use additional plugin class loader factory.
skrzypo987
force-pushed
the
skrzypo/266-plugin-manager-rewrite-as-plugin-installer
branch
from
5a50036 to
9bea149
Compare
martint
left a comment
martint
left a comment
There was a problem hiding this comment.
Classes such as ResourceGroupManager, TypeRegistry, GlobalFunctionCatalog, etc., should not have to be aware of plugin infrastructure -- it should be the opposite.
|
In such case as I understand you, that means that we should have dedicated plugin installers per such entity. For example |
4 participants
Description
This PR rewrites the plugin installation process into smaller installers using mechanism from #16060. That way the logic of installing plugins i s moved into classes, where they are actually installed, and only guice bindings link the two together.
Additional context and related issues
Release notes
(x) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text: