couchbase: wait until all services are part of the config #3003
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| } | ||
| return true; | ||
| } catch (IOException ex) { | ||
| logger().error("Unable to parse response {}", rawConfig); |
There was a problem hiding this comment.
exception is swallowed here
bsideup
reviewed
Jul 17, 2020
| @SuppressWarnings("unchecked") | ||
| public boolean test(final String rawConfig) { | ||
| try { | ||
| Map<String, Object> parsedConfig = MAPPER.readValue(rawConfig, new TypeReference<Map<String, Object>>() {}); |
There was a problem hiding this comment.
Consider using readTree (and its path(String key) to have a null-safe access to sub-objects)
There was a problem hiding this comment.
I second this, no more casting to Map/List..
There was a problem hiding this comment.
I'll get it patched up shortly
daschl
force-pushed
the
wait-service-available
branch
from
f7caad2
to
848ae83
Compare
| @@ -168,7 +174,7 @@ protected void configure() { | |||
| .map("healthy"::equals) | |||
| .orElse(false); | |||
| } catch (IOException e) { | |||
| logger().error("Unable to parse response {}", response); | |||
| logger().error("Unable to parse response: " + response, e); | |||
There was a problem hiding this comment.
put back {} for response, Throwables can be the last argument to error() calls.
logger().error("Unable to parse response {}", response, e);
| } | ||
| return true; | ||
| } catch (IOException ex) { | ||
| logger().error("Unable to parse response: " + rawConfig, ex); |
There was a problem hiding this comment.
use {} placeholder for rawConfig
old habits die hard, eh? ;)
daschl
force-pushed
the
wait-service-available
branch
from
848ae83
to
3e538bc
Compare
aaronjwhiteside
approved these changes
Jul 20, 2020
|
@daschl can please have the conflict resolved? |
|
@daschl bump! |
This changeset adds a predicate to the wait strategy to make sure that it not only returns a 200, but also that every enabled service is actually already exposed in the config. Since we are polling the server during bootstrap here, not all of them might show up at the same time. Also, while not contributing to the fix we poll the terse bucket http config "b" instead of the verbose one "buckets" since it is a little more efficient on the server side and actually the config the client internally works with. fixes testcontainers#2993
daschl
force-pushed
the
wait-service-available
branch
from
3e538bc
to
c6598aa
Compare
|
@aaronjwhiteside conflicts resolved and rebased with master. |
|
@aaronjwhiteside on it 👍 |
This was referenced Mar 13, 2021
This was referenced Mar 18, 2021
sd-yip
added a commit
to sd-yip/testcontainers-java
that referenced
this pull request
May 12, 2021
…stcontainers#3003)" This reverts commit ed8386c.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This changeset adds a predicate to the wait strategy to make sure
that it not only returns a 200, but also that every enabled service
is actually already exposed in the config. Since we are polling
the server during bootstrap here, not all of them might show up
at the same time.
Also, while not contributing to the fix we poll the terse bucket
http config "b" instead of the verbose one "buckets" since it is
a little more efficient on the server side and actually the config
the client internally works with.
fixes #2993