Skip to content

handle "stop" event#9019

Merged
ndeloof merged 1 commit into
docker:v2from
ndeloof:8523
Dec 9, 2021
Merged

handle "stop" event#9019
ndeloof merged 1 commit into
docker:v2from
ndeloof:8523

Conversation

@ndeloof

@ndeloof ndeloof commented Dec 8, 2021

Copy link
Copy Markdown
Contributor

What I did
This is an alternative to #9018
catching the stop event, we can stop watching a container and consider it "done"

this include moving convert to a non-main package as this prevent debugger to start in Goland

Related issue
fixes: #8523

(not mandatory) A picture of a cute animal, if possible in relation with what you did
image

(he won't run anymore)

Comment thread pkg/compose/start.go Outdated
@ulyssessouza

Copy link
Copy Markdown
Contributor

@ndeloof gofmt

@ndeloof
ndeloof force-pushed the 8523 branch 3 times, most recently from 7d39396 to 91509e3 Compare December 9, 2021 08:51
@ulyssessouza
ulyssessouza self-requested a review December 9, 2021 09:47

@ulyssessouza ulyssessouza left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just the spammer line

Comment thread pkg/compose/start.go Outdated
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>

@ulyssessouza ulyssessouza left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. But I just noticed we have no tests for this function :(

@1ace

1ace commented Dec 24, 2021

Copy link
Copy Markdown

@ulyssessouza:

But I just noticed we have no tests for this function :(

FYI, I posted a non-interactive (no manual step) reproducer for this bug here: #9003 (comment)

I'm not familiar with your CI setup but I expect it should be easy enough to add that docker-compose.yml file somewhere and run the command I provided from your CI, avoiding this bug from being introduced again at some point in the future 🙂

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.

Docker-compose no longer gracefully stops

3 participants