
|
If you were logged in you would be able to see more operations.
|
|
|
|
The plugin API for manual discovery is not good - it is crammed into the ResourceDiscoveryComponent.discoverResources() method as add-on functionality. A plugin that wants to support both auto-discovery and manual discovery has to use code like the following in its impl of discoverResources():
List<Configuration> pluginConfigs = discoveryContext.getPluginConfigurations();
if (pluginConfigs.isEmpty()) {
// autodiscover resources
} else {
// process manually added resources
}
So the plugin writer must know that 1) the PC will invoke discoverResources() separately for autodiscovery and manual adds, and 2) plugin configs being non-empty signifies a manual add. Also, for some reason a List<Configuration> is passed, even though the PC never passes more than one plugin config in (because the invocation of discoverResources() corresponds to a user-initiated manual add of a single resource).
I propose splitting out manual-add into its own method:
DiscoveredResourceDetails processManuallyAddedResource(ResourceDiscoveryContext<T> context, Configuration pluginConfig)
The method would live in a separate ManualAddDiscoveryFacet interface, which would be implemented by the ResourceDiscoveryComponents of resource types that include supportsManualAdd="true" in their plugin descriptors.
Additionally, the following methods would be removed from ResourceDiscoveryContext:
List<Configuration> getPluginConfigurations()
List<ProcessScanResult> getAutoDiscoveredProcesses()
getPluginConfigurations() would be removed, because the plugin config for a manual add would now be passed directly to processManuallyAddedResource(). getAutoDiscoveredProcesses() would be removed because it is only relevant to discoverResources() , not processManuallyAddedResource(), and it is more of a direct input for discoverResources() than contextual info anyway. So the new siganture for discoverResources() would be:
Set<DiscoveredResourceDetails> discoverResources(ResourceDiscoveryContext<T> context, List<ProcessScanResult> autoDiscoveredProcesses)
The proposed changes to the PC are fairly minimal - maybe 1/2 hour of work. However, this would require refactoring all of our plugins to use the updated API, which would take 2-3 hours. However, I think it is worthwhile, since it makes for a much more intuitive and elegant API for plugin developers.
|
|
Description
|
The plugin API for manual discovery is not good - it is crammed into the ResourceDiscoveryComponent.discoverResources() method as add-on functionality. A plugin that wants to support both auto-discovery and manual discovery has to use code like the following in its impl of discoverResources():
List<Configuration> pluginConfigs = discoveryContext.getPluginConfigurations();
if (pluginConfigs.isEmpty()) {
// autodiscover resources
} else {
// process manually added resources
}
So the plugin writer must know that 1) the PC will invoke discoverResources() separately for autodiscovery and manual adds, and 2) plugin configs being non-empty signifies a manual add. Also, for some reason a List<Configuration> is passed, even though the PC never passes more than one plugin config in (because the invocation of discoverResources() corresponds to a user-initiated manual add of a single resource).
I propose splitting out manual-add into its own method:
DiscoveredResourceDetails processManuallyAddedResource(ResourceDiscoveryContext<T> context, Configuration pluginConfig)
The method would live in a separate ManualAddDiscoveryFacet interface, which would be implemented by the ResourceDiscoveryComponents of resource types that include supportsManualAdd="true" in their plugin descriptors.
Additionally, the following methods would be removed from ResourceDiscoveryContext:
List<Configuration> getPluginConfigurations()
List<ProcessScanResult> getAutoDiscoveredProcesses()
getPluginConfigurations() would be removed, because the plugin config for a manual add would now be passed directly to processManuallyAddedResource(). getAutoDiscoveredProcesses() would be removed because it is only relevant to discoverResources() , not processManuallyAddedResource(), and it is more of a direct input for discoverResources() than contextual info anyway. So the new siganture for discoverResources() would be:
Set<DiscoveredResourceDetails> discoverResources(ResourceDiscoveryContext<T> context, List<ProcessScanResult> autoDiscoveredProcesses)
The proposed changes to the PC are fairly minimal - maybe 1/2 hour of work. However, this would require refactoring all of our plugins to use the updated API, which would take 2-3 hours. However, I think it is worthwhile, since it makes for a much more intuitive and elegant API for plugin developers.
|
Show » |
| There are no comments yet on this issue.
|
|