Bug 535849 - (RHQ-25) fix manual discovery plugin API
fix manual discovery plugin API
Status: CLOSED CURRENTRELEASE
Product: RHQ Project
Classification: Other
Component: Plugins (Show other bugs)
1.2
All All
medium Severity medium (vote)
: ---
: ---
Assigned To: Ian Springer
Sudhir D
http://jira.rhq-project.org/browse/RH...
: Task
Depends On:
Blocks: RHQ-2439
  Show dependency treegraph
 
Reported: 2008-03-04 11:21 EST by Ian Springer
Modified: 2013-08-05 20:33 EDT (History)
2 users (show)

See Also:
Fixed In Version: 2.4
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-08-12 12:47:57 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Ian Springer 2008-03-04 11:21:00 EST
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.
Comment 1 Ian Springer 2009-09-23 13:16:16 EDT
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.
Comment 2 Red Hat Bugzilla 2009-11-10 16:05:43 EST
This bug was previously known as http://jira.rhq-project.org/browse/RHQ-25
Comment 3 Corey Welton 2010-03-29 13:32:29 EDT
Mass-move to ON_QA
Comment 5 Sudhir D 2010-06-11 09:32:35 EDT
I have tested this with a test plugin and it gets added successfully. 

Marking this bug as verified.
Comment 6 Corey Welton 2010-08-12 12:47:57 EDT
Mass-closure of verified bugs against JON.

Note You need to log in before you can comment on or make changes to this bug.