Allow loading of config maps to be retried on failure #648
Conversation
…figurable wrapper around spring-retry
sagacity
force-pushed
the
configmap-retry-policy
branch
from
8bdb45c
to
1f875d4
Compare
|
A test is failing on CircleCI, but this seems unrelated to my change (a |
|
Any updates on this? |
There was a problem hiding this comment.
Awesome, thanks for this PR!
I added support for reading ConfigMaps with the Kuberenetes Java client which unfortunately has caused a bunch of merge conflicts. Could we resolve those and maybe take a look at adding the same type of retry logic there as well?
Also could the same functionality be added for secrets?
| @@ -49,8 +53,10 @@ | |||
|
|
|||
| @Bean | |||
| @ConditionalOnProperty(name = "spring.cloud.kubernetes.config.enabled", matchIfMissing = true) | |||
| public ConfigMapPropertySourceLocator configMapPropertySourceLocator(ConfigMapConfigProperties properties) { | |||
| return new ConfigMapPropertySourceLocator(this.client, properties); | |||
| public ConfigMapPropertySourceLocator configMapPropertySourceLocator(ConfigMapConfigProperties properties, | |||
There was a problem hiding this comment.
Instead of the Optional here I think it might be cleaner if you used @ConditionalOnMissingClass
|
@ryanjbaxter I am currently on paternity leave but hopefully I can take another look in the new year! |
|
Great! |
|
@ryanjbaxter I'm back from leave but unfortunately not in the position to continue work. @vy @breun any chance of picking this up? |
|
No worries, I can probably take a look in a few weeks. |
|
@ryanjbaxter it seems this can be closed also, as we already have retry thx to the excellent effort from @isikerhan |
I'm trying to solve #441. There was already a pull request (#446) for this, but it stalled. Instead of rebasing that PR I've tried to simply add a wrapper around spring-retry, since the discussion in that pull request seemed to indicate this is a valid approach.
Can you confirm this approach would be ok for you? If so, I will spend some more time on the PR adding appropriate tests. Otherwise I'm open to changing directions.
The reason I didn't try to abstract away
spring-retryis that it's a fairly well known project in the Spring ecosystem and it felt unnecessary to hide that. The configuration is set up so that if the dependency is not available retry logic will simply be unavailable and the old behaviour will be used.