-
Notifications
You must be signed in to change notification settings - Fork 16
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
service waiting logic #2
Conversation
09d14f6
to
3f45a3c
Compare
docker_services_cli/services.py
Outdated
READYNESS_CHECKS = { | ||
"es": es_healthcheck, | ||
"postgresql": postgresql_healthcheck, | ||
"mysql": mysql_healthcheck, | ||
"redis": redis_healthcheck, | ||
} | ||
"""Readyness check functions module path, as 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.
In my opinion, I think that this code belongs to the wait_for_services
implementation, it is just what in other languages would be a switch/case etc.. so no need for the comment.
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.
Fine. I am still not convinced because by that line of thought we should have the services configuration (e.g. MYSQL
) also in the services.py
file.
In any case to be able to keep going I opened an question issue #4 and removed the comment.
@diegodelemos sample of usage running here: https://travis-ci.org/github/ppanero/invenio-records-rest/builds/721264883 (I checked the |
|
||
|
||
def services_up(services, filepath=DOCKER_SERVICES_FILEPATH): | ||
def services_up(services, filepath=DOCKER_SERVICES_FILEPATH, wait=True): |
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.
services
needs a default (which should be all services). If one does:
$ docker-services-cli up
The wait_for_services
doesn't do anything because services
is an empty 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.
LGTM, solution for #2 (comment) discussed over videocall.
Example execution with ES7 (slowest):