Hey Appy,
I think Enis essentially copied what I was originally trying to do. Up
in Avatica[1], I made a similar API Maven module which just had
interfaces. The implementation was them split up into a different module
to avoid the implementation details of the API (specifically, using
Dropwizard3) from "infecting" anyone who just wanted the API.
Implementations of the metrics API could be picked up via ServiceLoader.
So, here in HBase, our "hbase-metrics" module is really an
implementation of hbase-metrics-api for dropwizard-metrics 3.2.1 (the
naming just loses that detail).
In Avatica, I wanted to make sure that people could use a metrics
library implementation of their choosing (e.g. tied to whatever kind of
container or app-server you're deploying Avatica). In HBase, that isn't
such a concern -- we publish via Metrics2 and that's it.
That said, I could see a place where this separation enables some extra
functionality with less headache, but I, admittedly, don't have a
concrete use-case behind it.
- Josh
[1] https://github.com/apache/calcite-avatica
On 10/24/17 6:09 PM, Apekshit Sharma wrote:
Hi
Am confused why do we have this weird circular dependency between the two
modules: hbase-metrics-api & hbase-metrics. And since maven doesn't allow
it explicitly, this was tucked and hidden by using reflection?
MetricRegistriesImpl[hbase-metrics] implements
MetricRegistries[hbase-metrics-api].
MetricRegistriesLoader[hbase-metrics-api] depends on this implementation,
MetricRegistriesImpl, because it instantiates it via reflection
<https://github.com/apache/hbase/blob/eee3b0180ead73c09b33f9583bfee9c01bc3aed2/hbase-metrics-api/src/main/java/org/apache/hadoop/hbase/metrics/MetricRegistriesLoader.java#L39>
[1].
Can we just move all interfaces and default implementation together into
single module. But i like the idea of keeping implementations in separate
package with "impl" explicitly in package name.
[1]
https://github.com/apache/hbase/blob/eee3b0180ead73c09b33f9583bfee9c01bc3aed2/hbase-metrics-api/src/main/java/org/apache/hadoop/hbase/metrics/MetricRegistriesLoader.java#L39
-- Appy
