Skip to content
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

Handle Ctrl+C when images are being pulled when dojo action is: run #15

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

xmik
Copy link
Member

@xmik xmik commented Jan 4, 2020

This PR fixes #1.
For the driver: docker - a SIGINT is sent to the current process group.
For the driver: docker-compose - Dojo will first explicitly invoke the pull action.

Besides, this PR also fixes a minor bug which happened on handling signals for the driver: docker-compose when a default container was not created. Dojo used to panic then, now it will proceed with cleaning.

@tomzo
Copy link
Member

tomzo commented Jan 4, 2020

The change in the run action for docker-compose driver is that now Dojo first explicitly invokes the pull action
and then it invokes the run action.

I don't think this is acceptable solution. It should not pull images before each run. The cases where it might have bad consequences are:

  • when there is docker image tag such as :latest
  • or when there are local docker images referred in the configuration, such as my-local-dojo or my-not-yet-released-server:0.0.1.

Then pull would either get something that developer does not want on their workstation yet, or completely fail in the later case when pulling my-not-yet-released-server:0.0.1.

We should keep the default docker behavior for runs - pull only when image is missing locally.

Rather than calling pull explicitly, I think you should handle the signals to simply interrupt the docker-compose run that is executed.

@xmik
Copy link
Member Author

xmik commented Jan 4, 2020

Good points, I will rethink the implementation for the docker-compose driver.

@xmik xmik force-pushed the master branch 6 times, most recently from 01e664d to 4387efa Compare February 20, 2022 10:27
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.

Ctrl+C does not stop docker pull when dojo action is: run
2 participants