The more I think about it, the more I get the feeling that if we can't away with execute(ServiceReference) in the executor api, this should be the only method of that service. The two others can easily be replaced by using the HCFilter and then calling the method above for each service reference. It's just one line more code, but this would completely remove the need to add meta information about the HC in the result and make the whole implementation easier.
WDYT? Carsten 2013/12/30 Carsten Ziegeler <cziege...@apache.org> > 2013/12/27 Bertrand Delacretaz <bdelacre...@apache.org> > >> Hi Carsten and Georg, >> >> I was going to say that I still disagree, but being outnumbered I'd >> let it go if it's just about a single method that I don't like...and >> then I reviewed the org.apache.sling.hc.api package once again and >> it's worse than I thought. >> > The problem is really, if we don't have a method with this signature how > can the jmx implementation make use of the executor while the executor is > still able to cache the result, do async processing etc. ? I don't see any > other solution atm, apart from moving the jmx implementation into the core. > > >> >> You guys both say you don't want metadata in APIs...and the >> HealthCheckResult interface that you added has these two methods >> (which make sense BTW): >> >> String getHealthCheckName(); >> List<String> getHealthCheckTags(); >> >> So a HC is not brave enough to have this in its API, but a Result >> needs them? This clearly demonstrates that we need a >> HealthCheckMetadata interface that provides those values. >> >> I think these are different concerns, a service(!) does not need an api > to expose the metadata because it's already in the service registration. > When we return a set of results, there need to be way of correlation > between a single result and an actual check, otherwise you get a bunch of > results without any additional information about the check. > The way out of this, would be to just have a single method > execute(ServiceReference) in the executor :) - but as soon as we allow > execution of more than a single HC we need this additional information. > > >> On top of that, the Result throws UnsupportedException on these >> methods...how ugly is that, why can't it provide these values from the >> HealthCheck that was used to create it? >> > I haven't looked deeply at this part yet... > >> >> To me this smells of implementation-driven APIs, we are used to better >> design in Sling. >> >> As pointed out, I'm all ears for a better solution of the executor > interface that allows us to get the above mentioned features. And as > pointed out below, if we simplify the executor service to a single method, > we don't need to make any changes to Result or add any additional > information. > > Carsten > > What do others think? >> >> -Bertrand >> >> [1] >> >> >> https://svn.apache.org/repos/asf/sling/trunk/bundles/extensions/healthcheck/core/src/main/java/org/apache/sling/hc/api >> > > > > -- > Carsten Ziegeler > cziege...@apache.org > -- Carsten Ziegeler cziege...@apache.org
