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

support attach #10700

Merged
merged 1 commit into from Jul 10, 2023
Merged

support attach #10700

merged 1 commit into from Jul 10, 2023

Conversation

ndeloof
Copy link
Contributor

@ndeloof ndeloof commented Jun 13, 2023

What I did
Add support for attach in service config, which can be overridden by command line --attach --no-attach

Related issue
closes #10695

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

@codecov
Copy link

codecov bot commented Jun 13, 2023

Codecov Report

Patch coverage: 60.00% and project coverage change: +0.01 🎉

Comparison is base (8a64ab5) 59.15% compared to head (46d936c) 59.17%.

Additional details and impacted files
@@            Coverage Diff             @@
##               v2   #10700      +/-   ##
==========================================
+ Coverage   59.15%   59.17%   +0.01%     
==========================================
  Files         115      115              
  Lines        9853     9869      +16     
==========================================
+ Hits         5829     5840      +11     
- Misses       3434     3437       +3     
- Partials      590      592       +2     
Impacted Files Coverage Δ
pkg/utils/set.go 34.61% <25.00%> (+34.61%) ⬆️
cmd/compose/up.go 68.51% <65.00%> (-1.49%) ⬇️
cmd/compose/compose.go 72.25% <100.00%> (ø)

... and 3 files with indirect coverage changes

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

@ndeloof ndeloof marked this pull request as ready for review June 20, 2023 15:26
@ndeloof ndeloof requested review from a team, nicksieger, StefanScherer, ulyssessouza, glours, milas and laurazard and removed request for a team June 20, 2023 15:26
Copy link
Member

@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.

I know this is still waiting on the compose-go side, will re-review once that's ready

Comment on lines 171 to 174
if err := project.WithServices(attachTo, func(s types.ServiceConfig) error {
if s.Attach == nil || *s.Attach {
attachTo = append(attachTo, s.Name)
}
Copy link
Member

Choose a reason for hiding this comment

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

If upOptions.attach is not empty, won't this add duplicates?

cmd/compose/up.go Show resolved Hide resolved
@ndeloof ndeloof force-pushed the attach branch 4 times, most recently from 62100a9 to 53f2f00 Compare July 10, 2023 11:52
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
@glours glours merged commit ee6aeed into docker:v2 Jul 10, 2023
25 checks passed
@ndeloof ndeloof deleted the attach branch July 10, 2023 13:53
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.

add --no-attach option to the docker-compose.yml config
3 participants