Use IsServing to determine if c8d client is ready#41228
Merged
Conversation
cpuguy83
force-pushed
the
better_event_wait
branch
from
July 17, 2020 20:41
dc5bd1b to
f65d271
Compare
thaJeztah
added
area/runtime
cpuguy83
force-pushed
the
better_event_wait
branch
from
July 28, 2020 17:17
58ada30 to
41537e2
Compare
thaJeztah
reviewed
Jul 29, 2020
Comment on lines
726
to
731
Member
There was a problem hiding this comment.
Wondering if if would be useful to return the error (and log it?) instead of returning a boolean; so err == nil means "yup, it's up now", and err != nil means something went wrong
Member
Author
There was a problem hiding this comment.
I can do a better job at checking the error from IsServing and logging cases that are not context related.
I don't think returning an error here is what we want though, since this should loop forever until the condition is true or the context is cancelled.
Member
|
@tonistiigi @AkihiroSuda PTAL |
Instead of sleeping an arbitrary amount of time, using the client to tell us when it's ready so we can start processing events sooner. Signed-off-by: Brian Goff <[email protected]>
The event subscriber can only be cancelled by cancelling the context. In the case where we have to restart event processing we are never cancelling the old subscribiption. Signed-off-by: Brian Goff <[email protected]>
cpuguy83
force-pushed
the
better_event_wait
branch
from
August 12, 2020 17:09
41537e2 to
906007f
Compare
Member
|
Interesting failure; shouldn't $ docker rm -fv 798c9471b695
Error: No such container: 798c9471b695
$ echo $?
1Apparently we don't ignore that (wondering if that's a bug / oversight, or if it was "by design" and not something we could change) |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Instead of sleeping an arbitrary amount of time, using the client to
tell us when it's ready so we can start processing events sooner.