Bug 535849 (RHQ-25)
| Summary: | fix manual discovery plugin API | ||
|---|---|---|---|
| Product: | [Other] RHQ Project | Reporter: | Ian Springer <ian.springer> |
| Component: | Plugins | Assignee: | Ian Springer <ian.springer> |
| Status: | CLOSED CURRENTRELEASE | QA Contact: | Sudhir D <sdharane> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | 1.2 | CC: | ccrouch, cwelton |
| Target Milestone: | --- | Keywords: | Task |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | All | ||
| URL: | http://jira.rhq-project.org/browse/RHQ-25 | ||
| Whiteboard: | |||
| Fixed In Version: | 2.4 | Doc Type: | Bug Fix |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2010-08-12 16:47:57 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
| Bug Depends On: | |||
| Bug Blocks: | 535778 | ||
r5212 - add the new API (a ManualAddFacet discovery facet with a discoverResource(Configuration pluginConfig, ResourceDiscoveryContext context) method) and deprecate the old API (the context.getPluginConfigurations() method). NOTE: The discoverResources() method signature has not been changed. This bug was previously known as http://jira.rhq-project.org/browse/RHQ-25 Mass-move to ON_QA I have tested this with a test plugin and it gets added successfully. Marking this bug as verified. Mass-closure of verified bugs against JON. |
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.