-
Notifications
You must be signed in to change notification settings - Fork 414
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
Allow dockerize to wait for network dependencies before main command #23
Conversation
go func() { | ||
defer wg.Done() | ||
for { | ||
_, err := http.Get(u.String()) |
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.
Seems like this should check to make sure a 200
response is returned. What if a 404
or 500
is returned by the dependency?
You an also specify a timeout using a custom http.Client
and/or Transport
.
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.
Good point, I originally interpreted the err to mean non-20x. This is fixed.
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.
RE: timeout. I did consider this, but felt the added complexity wasn't going to give us much given we check the timeout of the entire set of dependencies anyway. Let me know if you would prefer it the other way.
afc2db9
to
0ab5bf8
Compare
ping @jwilder looks like this PR has been updated; ptal 👍 |
Allow dockerize to wait for network dependencies before main command
Thanks! 🎉 |
Thanks @mefellows! Nice feature idea. |
No worries at all - do I need to do anything to get this 'released' or will you look after that in due course? |
@mefellows I just created a new |
Awesome, thanks! |
It is common when using tools like Docker Compose to depend on services in other linked containers, however oftentimes relying on links is not enough - whilst the container itself may have started, the service(s) within it may not yet be ready - resulting in shell script hacks to work around race conditions.
This PR gives dockerize the ability to wait for services on a specified protocol (
tcp
,tcp4
,tcp6
,http
, andhttps
) before starting the main application:Note the
timeout
is optional and defaults to10s
.I've found this to be particularly useful when using Docker Compose as a test harness, where one of the containers needs to test another. Instead of
netcat
ing my way around the problem, I can just wrap the command usingdockerize
.I also fixed a few minor grammatical/spelling mistakes.
Related links: