-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
couchbase: wait until all services are part of the config #3003
Conversation
} | ||
return true; | ||
} catch (IOException ex) { | ||
logger().error("Unable to parse response {}", rawConfig); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exception is swallowed here
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using readTree
(and its path(String key)
to have a null-safe access to sub-objects)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I second this, no more casting to Map/List..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll get it patched up shortly
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use {}
placeholder for rawConfig
old habits die hard, eh? ;)
848ae83
to
3e538bc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not that I have any sway here, but LGTM
@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
3e538bc
to
c6598aa
Compare
@aaronjwhiteside conflicts resolved and rebased with master. |
@aaronjwhiteside on it 👍 |
…stcontainers#3003)" This reverts commit ed8386c.
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