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

Fix pull --parallel and --no-parallel deprecation warnings missing #12555

Merged
merged 1 commit into from
Feb 20, 2025

Conversation

maxproske
Copy link
Contributor

What I did

I believe --no-parallel was mistakenly bound to opts.parallel and as a result, the deprecation warning in the PreRunE hook was never printed. Fixed the binding, and added a matching deprecation warning for --parallel.

Before (No deprecation warnings in stderr):

# compose.yaml
services:
  app:
    image: nginx:latest
$ docker compose pull --parallel=false
[+] Pulling 1/1
 ✔ app Pulled

$ docker compose pull --no-parallel
[+] Pulling 1/1
 ✔ app Pulled

After:

$ ./docker-compose pull --parallel=false
option '--parallel' is DEPRECATED and will be ignored.
[+] Pulling 1/1
 ✔ app Pulled

$ ./docker-compose pull --no-parallel
option '--no-parallel' is DEPRECATED and will be ignored.
[+] Pulling 1/1
 ✔ app Pulled

@maxproske maxproske requested a review from a team as a code owner February 17, 2025 06:52
@maxproske maxproske requested review from ndeloof and glours February 17, 2025 06:52
@@ -64,7 +67,7 @@ func pullCommand(p *ProjectOptions, dockerCli command.Cli, backend api.Service)
cmd.Flags().BoolVar(&opts.includeDeps, "include-deps", false, "Also pull services declared as dependencies")
cmd.Flags().BoolVar(&opts.parallel, "parallel", true, "DEPRECATED pull multiple images in parallel")
flags.MarkHidden("parallel") //nolint:errcheck
cmd.Flags().BoolVar(&opts.parallel, "no-parallel", true, "DEPRECATED disable parallel pulling")
cmd.Flags().BoolVar(&opts.noParallel, "no-parallel", true, "DEPRECATED disable parallel pulling")
Copy link
Member

Choose a reason for hiding this comment

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

If it's attached to "no parallel", should the default be reversed to false @ndeloof ?

Copy link
Contributor

Choose a reason for hiding this comment

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

indeed. Ignored anyway

@glours glours merged commit 4e593ed into docker:main Feb 20, 2025
26 checks passed
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.

4 participants