Hi Josh You're right that naming doesn't make it clear, but Enis added a really great description in README of hbase-metrics-api to explain what's in each module. However it's not obvious why are we splitting things into separate module, as opposed to say, separating implementation in a separate "sub" package.
I don't know Avatica, but as you clearly state, one of the requirements was being able to plug in different implementations. Using service provider clearly makes sense in that case, so does keeping the implementation in a separate jar to avoid loading it if not needed. But in case of HBase, even if we have multiple implementations (hypothetically, because it doesn't seem probable), since the only user will be HBase and all modules will always be in classpath, it doesn't really get us anything. Right? I dug into this code first time today and just learnt about service providers, so i may be off, in which case please correct me. Thank you! On Tue, Oct 24, 2017 at 8:25 PM, Josh Elser <els...@apache.org> wrote: > 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/eee3b0180ead73c09b33f9 >> 583bfee9c01bc3aed2/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/eee3b0180ead73c09b33f95 >> 83bfee9c01bc3aed2/hbase-metrics-api/src/main/java/org/ >> apache/hadoop/hbase/metrics/MetricRegistriesLoader.java#L39 >> >> -- Appy >> >> -- -- Appy
