Support to use classes registered in bootstrap context #325
Conversation
| () -> null, () -> null); | ||
| InstanceSerializer<ZookeeperInstance> serializer = new JsonInstanceSerializer<>(ZookeeperInstance.class); | ||
| ZookeeperDiscoveryProperties discoveryProperties = binder.bind(ZookeeperDiscoveryProperties.PREFIX, Bindable | ||
| CuratorFramework curatorFramework = context.getOrElse(CuratorFramework.class, CuratorFactory.curatorFramework(properties, retryPolicy, Stream::of, |
There was a problem hiding this comment.
Would we ever reach the orElse here? I mean even before the lambda gets registered, that will later create this ZookeeperFunction, the code has already called CuratorFactory.registerCurator. (I may be missing some use-case though.)
There was a problem hiding this comment.
I was probably being over cautious here, there is no harm though.
There was a problem hiding this comment.
Apart from being a bit confusing to readers (who may expect this line to be creating their CuratorFramework instance), won't this getOrElse code always end up running the CuratorFactory.curatorFramework(...) call? If so, that would always build another CuratorFramework instance, start it, and block while it connects (or fails to do so), only to then throw it away (without a close() call). When this instance can't connect, it may also write a considerable amount of misleading logs, that could become red herrings when debugging some issue. Not sure if the curatorFramework(...) method could also throw an exception or not.
There was a problem hiding this comment.
It would only call CuratorFactory.curatorFramework if there was not one already present in the BootstrapContext.
I don't see how it would start 2...
There was a problem hiding this comment.
I may be missing something, but my thinking was that:
- the parameters of this
getOrElsecall have to be evaluated first by the JVM, meaning thatCuratorFactory.curatorFramework(...)is executed, building & starting the newCuratorFramework, then - the JVM passes this new
CuratorFrameworkintogetOrElseas theotherparameter, then getOrElseitself gets executed, where it will simply ignore theotherparameter when it realizes that there's already aCuratorFrameworkin theBootstrapContext.
Fixes #324