Fix 1641 part 3 : make fabric8 discovery client cacheable (3) #2064
Conversation
Signed-off-by: wind57 <[email protected]>
…overy-client-cacheable
…overy-client-cacheable
Signed-off-by: wind57 <[email protected]>
Signed-off-by: wind57 <[email protected]>
wind57
changed the title
Signed-off-by: wind57 <[email protected]>
| @Documented | ||
| @Inherited | ||
| @Conditional(ConditionalOnDiscoveryCacheableBlockingDisabled.OnDiscoveryCacheableBlockingDisabled.class) | ||
| public @interface ConditionalOnDiscoveryCacheableBlockingDisabled { |
There was a problem hiding this comment.
introduce 4 new conditionals (2 for blocking, 2 for reactive), to let users have the option to select a cacheable or a non-cacheable discovery client. The default one is non-cacheable.
| /** | ||
| * @author wind57 | ||
| */ | ||
| abstract class Fabric8AbstractBlockingDiscoveryClient implements DiscoveryClient { |
There was a problem hiding this comment.
all the existing code AS-IS, from the DiscoveryClient has moved in here, without any kind of changes. Now both cacheable and non-cacheable discovery clients extend it.
| } | ||
|
|
||
| @Override | ||
| @Cacheable("fabric8-blocking-discovery-services") |
There was a problem hiding this comment.
cacheable services and instances for the blocking discovery client
| } | ||
|
|
||
| @Override | ||
| @Cacheable("fabric8-reactive-discovery-instances") |
There was a problem hiding this comment.
cacheable reactive discovery client
| private final KubernetesNamespaceProvider namespaceProvider; | ||
|
|
||
| private final Predicate<Service> predicate; | ||
| final class Fabric8DiscoveryClient extends Fabric8AbstractBlockingDiscoveryClient { |
There was a problem hiding this comment.
this client does not change at all, it just delegates the calls now, but its exactly the same code
There was a problem hiding this comment.
Why override these methods just to call super?
There was a problem hiding this comment.
because both cacheable and non-cacheable implementations delegate to the same super, so they do the exact thing, with the difference that one is cacheable capable, and one is not
There was a problem hiding this comment.
Right but in the non-cachable versions where we don't have to add the @Cachable annotation, why do we need to override these methods in the class?
There was a problem hiding this comment.
OMG! you're right! thank you :)
|
|
||
| @Bean | ||
| @ConditionalOnMissingBean | ||
| @ConditionalOnDiscoveryCacheableBlockingDisabled |
There was a problem hiding this comment.
you can now select cacheable or non-cacheable discovery client
|
|
||
| Fabric8ReactiveDiscoveryClient(Fabric8DiscoveryClient fabric8DiscoveryClient) { | ||
| this.fabric8DiscoveryClient = fabric8DiscoveryClient; | ||
| super(fabric8DiscoveryClient); |
There was a problem hiding this comment.
same as for the blocking implementation, the logic does not change at all here, it just delegates
| @Bean | ||
| @ConditionalOnMissingBean | ||
| @ConditionalOnDiscoveryCacheableReactiveEnabled | ||
| Fabric8CacheableReactiveDiscoveryClient fabric8CacheableReactiveDiscoveryClient(KubernetesClient client, |
There was a problem hiding this comment.
reactive clients now can also be cacheable or non-cacheable
|
@ryanjbaxter this starts to add cacheable discovery clients. First we do it for fabric8, have PRs in motion for the rest too. Ready to look at now. |
|
We should add some documentation around this, will that come in a later PR? |
|
yes, documentation will be added as soon as all 3 clients have this option |
Signed-off-by: wind57 <[email protected]>
3 participants
No description provided.