Conversation
By enumerating features that were installed in the server once plugin was loaded
Verification already happens when all plugins are installed
wendigo
force-pushed
the
user/serafin/plugin-loading-report
branch
from
ab8d171 to
0bde0f5
Compare
| log.info("Loaded plugin %s (%s) with features:", COLORS.colored(features.pluginClass().getSimpleName(), PURPLE), features.loadingTime().convertToMostSuccinctTimeUnit()); | ||
| for (Feature feature : Feature.values()) { | ||
| List<String> names = features.names(feature); | ||
| if (!names.isEmpty()) { | ||
| log.info(" %s: %s", feature.getDescription(), Joiner.on(", ").join(names)); |
There was a problem hiding this comment.
Those 2 logs should be single log call (one line)
|
|
||
| for (ExchangeManagerFactory exchangeManagerFactory : plugin.getExchangeManagerFactories()) { | ||
| log.info("Registering exchange manager %s", exchangeManagerFactory.getName()); | ||
| builder.withFeature(Feature.EXCHANGE_MANAGER, exchangeManagerFactory.getName()); |
There was a problem hiding this comment.
So Feature enum is more FeatureType
also, maybe feature → component, extension, ... any other options concidered?
| void loadPlugins(); | ||
|
|
||
| void installPlugin(Plugin plugin); | ||
| InstalledFeatures installPlugin(Plugin plugin); |
There was a problem hiding this comment.
InstalledFeatures → PluginReflection, PluginInfo
it's not just "set of features" this plugin has, it includes other info like plugin main class (or installation time)
| void installPlugin(Plugin plugin); | ||
| InstalledFeatures installPlugin(Plugin plugin); | ||
|
|
||
| record InstalledFeatures(Class<? extends Plugin> pluginClass, Duration loadingTime, List<InstalledFeature> features) |
There was a problem hiding this comment.
it doesn't make sense to provide loadingTime to the caller installPlugin(Plugin) caller, because caller also knows how long it took.
it caller cares about how long a call took, let it measure that
| return new Builder(pluginClass); | ||
| } | ||
|
|
||
| public static class Builder |
There was a problem hiding this comment.
What do we need this builder for?
it looks the code would be equally readable without this class
| FUNCTION("functions"), | ||
| GROUP_PROVIDER("group providers"), | ||
| HEADER_AUTHENTICATOR("header authenticators"), | ||
| LANGUAGE_FUNCTION("language functions engines"), |
| bundle.getFunctions() | ||
| .stream() | ||
| .filter(function -> !function.isHidden()) | ||
| .forEach(metadata -> builder.withFeature(Feature.FUNCTION, metadata.getCanonicalName())); |
There was a problem hiding this comment.
Do we really want to log every function name during startup?
What was being informed about that is not longer there? |
2 participants
Description
Improves plugin loading by leaving only relevant information about plugin contents:
Additional context and related issues
Release notes
(x) This is not user-visible or is 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: