I am ok either way. if this doesn't have default implementation it can't be included into the 4.17.x releases and will have to wait for 4.18 (breaking change, e.g. pulsar implements MetadataClientDriver interface.)
OTOH if this is used for anything beyond tests I'd suggest default implementation to return false, but with this we can as well avoid default implementation and force people change code instead of failing at some random moment. On 2024/05/07 23:34:31 ZhangJian He wrote: > Hi, bookKeepers > > I've reviewed the PR that introduces `isDriverMetadataServiceAvailable`. I > have concerns about providing a default implementation that returns a > constant value like true, it's not a default interface like this. > > https://github.com/apache/bookkeeper/pull/4342#discussion_r1591761669 > > ``` > default E getFirstElement() { > return getElements().get(0); > } > > List<E> getElements() { > } > ``` > > - First we don't guarantee/make ABI compatible between minor releases > - Second, People who implement metadata drivers should implement this > correctly, it may lead to unintended behavior if it's not properly > overridden by all implementations. > > I'd suggest making this method abstract instead to avoid potential > misinterpretation. > > Thanks > ZhangJian He > Twitter: shoothzj > Wechat: shoothzj >
