add option to configure Consul check status considered as healthy#599
add option to configure Consul check status considered as healthy#599jackhammer2k wants to merge 5 commits intospring-cloud:mainfrom
Conversation
|
+1 for this issue. It would be awesome to get this merged :) |
|
Any comment from the maintainers? Does anyone know how to fix the failed build? |
|
It looks like your fork is some commits behind our master. Can you please merge our changes into your feature branch and try again? |
| * @see ConsulServer#ConsulServer(HealthService, java.util.Set) | ||
| */ | ||
| protected List<ConsulServer> transformResponse(List<HealthService> healthServices) { | ||
| protected List<ConsulServer> transformResponse(List<HealthService> healthServices, |
There was a problem hiding this comment.
To avoid changing the method signature I would use this.properties in the method rather than pass in the Set. You can add an overloaded method if you want to give the option to past in the Set
| private final Map<String, String> metadata; | ||
|
|
||
| public ConsulServer(final HealthService healthService) { | ||
| public ConsulServer(final HealthService healthService, |
There was a problem hiding this comment.
We need to maintain backwards compatibility so we need to leave this constructor as is (perhaps mark it as deprecated) and add a new one.
| /** | ||
| * List of status considered as healthy. Contains "passing" by default. | ||
| */ | ||
| private Set<Check.CheckStatus> statusConsideredAsHealthy = new HashSet( |
There was a problem hiding this comment.
We should document the new property
There was a problem hiding this comment.
You can add the explanation of this feature to docs/src/main/asciidoc/spring-cloud-consul.adoc.
docs/src/main/asciidoc/_configprops.adoc will be automatically generated for you so you don't need to make any changes here.
spencergibb
left a comment
spencergibb
left a comment
There was a problem hiding this comment.
I agree with my colleagues comments
* restore backwards-compatibility of changed classes
|
Looks like there is a sporadically failing test (I did not touch the failing module). Could you please restart the tests? |
6 participants
Instead of just considering check status "passing" as healthy we make the status configurable. That allows us to also consider e.g. "warning" as healthy.
Default is the current behavior that only allows "passing".