I think it's an improvement of the error handle in the metadata failure case. Not all cases need to implement it.
+1 to Andrey. That depends on which releases this feature wants to go. Yong On Fri, 17 May 2024 at 07:56, Andrey Yegorov <ayego...@apache.org> wrote: > 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 > > >
