Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix corner case when there's no container to attach to #10058

Merged
merged 1 commit into from
Dec 9, 2022

Conversation

ndeloof
Copy link
Contributor

@ndeloof ndeloof commented Dec 8, 2022

What I did
Fixed a corner case:
if one run docker compose up --scale helloworld=0 with a single-service compose file, no container is actually created (as expected) and watch will never detect application termination

Related issue
close #8752

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

@ndeloof ndeloof requested a review from glours December 8, 2022 17:58
@codecov
Copy link

codecov bot commented Dec 8, 2022

Codecov Report

Base: 77.02% // Head: 77.02% // No change to project coverage 👍

Coverage data is based on head (d3d30cf) compared to base (0234e13).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##               v2   #10058   +/-   ##
=======================================
  Coverage   77.02%   77.02%           
=======================================
  Files           2        2           
  Lines         235      235           
=======================================
  Hits          181      181           
  Misses         48       48           
  Partials        6        6           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@milas milas left a comment

Choose a reason for hiding this comment

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

Looks good - does this actually address the linked issue, however?

AFAICT that issue isn't about "no container to attach to" cases but <svc>=0 being a way to exclude a service from launching as part of up without having to list out every other service

@ndeloof
Copy link
Contributor Author

ndeloof commented Dec 9, 2022

#8752 indeeds report another issue, but the latest codebase doesn't behaves as reported (scale=0 is well supported now), still the reported scenario demonstrates as a deadlock when scale=0 is set (aka "regression").

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
@ndeloof ndeloof merged commit 8f991a2 into docker:v2 Dec 9, 2022
@ndeloof ndeloof deleted the corner_case branch December 9, 2022 09:03
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

up --scale not working on 2.0.1
2 participants