Skip to content

add depends_on in order to automatically await these services start on "docker-compose up" #359

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

piradata
Copy link

No description provided.

Copy link
Author

@piradata piradata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add depends_on in order to automatically await these services start on "docker-compose up"

@ryankwondev
Copy link

Look good to me! It looks like @hermanzdosilovic wrote that part because he intends for the rest of the containers to start after the Redis is fully booted, but if the first `docker-compose up -d' starts, it will take time to download the container from the Docker Hub, so if you use depends_on, you don't need to delay the service execution speed.

@piradata
Copy link
Author

yes, and them we could change this part on the changelog:

cd judge0-v1.13.0
docker-compose up -d db redis
sleep 10s
docker-compose up -d
sleep 5s

to be a simple docker-compose up -d

also, the deployment steps for a local intallation could be moved to readme, what do you think? took me a good time to find it on changelog heheh

@ryankwondev
Copy link

Agree with you. We can still find it in Line 50 in README.md,

Feel free to start with the [**FREE Basic Plan**](https://judge0.com/ce) on RapidAPI or [host it yourself](https://github.com/judge0/judge0/blob/master/CHANGELOG.md#deployment-procedure).
but I think accessibility is not that good. Could be better to use a separated paragraph.

@piradata
Copy link
Author

@ryankwondev its almost 2 years now, how is it? only remember about that cause got a message about a merge conflict

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants