It's not just about the metadata - it's about uniquely identifying a HC within the executor - the service object is not a good key, especially as we have no way to discard it from the cache. The service reference or the service id contained within the reference is a good key. And uniquely identifying (for long running hcs) and caching the hc result is for me the number one reason for the executor service.
We should not change the HC interface as this is an incompatible change and would break all existing implementations - as we've discussed during defining the first version of the API, there is no real need to have the name, tags etc. in the HC interface. Services executing an HC get the service anyway from the service registry together with these registration properties and it makes the implementation unnecessarily complicated. So we need something to uniquely identify a health check and as all metadata is optional we don't have anything to be used. Now, as I wrote in the issue, I'm fine with having an execute(String name) method which tries to execute a HC with the given name. This would require that a HC which can be executed by this service either has a tag or a name. On the other hand, if we go with ServiceReference we can execute all HCs through this service. And as Georg has pointed out, there are not many clients of this service anyway: its the jmx implementation and the web console. Carsten 2013/12/24 Bertrand Delacretaz <[email protected]> > Hi, > > About SLING-3278 - IMO execute(ServiceReference) at [1] is an > unnecessary leak of implementation details, it makes no sense at the > API level. It's HealthChecks that we are executing, not service > references. > > IIUC the root cause is that the HealthCheck API doesn't provide > metadata that it should: tags, name, optional JMX MBean name, unique > ID maybe, all these things clearly belong to a HealthCheck, so the API > should provide access to them, even if they are defined by service > properties. > > I suggest adding a HealthCheckMetadata interface to provide those > things (tags, name, optional things in a Map maybe), and add a > HealthCheck.getMetadata() method that returns this. That'll break > existing HealthCheck implementations, which we indicate by bumping up > the package version number, as this quite a new API it's better to fix > it now than later. > > -Bertrand > > [1] > https://svn.apache.org/repos/asf/sling/trunk/bundles/extensions/healthcheck/core/src/main/java/org/apache/sling/hc/api/HealthCheckExecutor.java > -- Carsten Ziegeler [email protected]
