Skip to content

Make '--env-file' option top-level only and fix failure with subcommands #6800

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.

Sign up for GitHub

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

Merged
rumpl merged 5 commits into from
Jul 25, 2019
Merged

Make '--env-file' option top-level only and fix failure with subcommands #6800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
99464d9
Handle environment file override within TopLevelCommand
KlaasH Jun 17, 2019
35eb404
Call TopLevelCommand's environment 'toplevel_environment'
KlaasH Jul 12, 2019
088a798
Fix typo in 'split_env' error message
KlaasH Jul 12, 2019
69c0683
Pass toplevel_environment to run_one_off_container
KlaasH Jul 12, 2019
413e5db
Add shell completions for --env-file option
KlaasH Jul 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 15 additions & 22 deletions compose/cli/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,11 @@ def __init__(self, project, options=None):
def project_dir(self):
return self.toplevel_options.get('--project-directory') or '.'

@property
def toplevel_environment(self):
environment_file = self.toplevel_options.get('--env-file')
return Environment.from_env_file(self.project_dir, environment_file)

def build(self, options):
"""
Build or rebuild services.
Expand Down Expand Up @@ -276,9 +281,7 @@ def build(self, options):
'--build-arg is only supported when services are specified for API version < 1.25.'
' Please use a Compose file version > 2.2 or specify which services to build.'
)
environment_file = options.get('--env-file')
environment = Environment.from_env_file(self.project_dir, environment_file)
build_args = resolve_build_args(build_args, environment)
build_args = resolve_build_args(build_args, self.toplevel_environment)

self.project.build(
service_names=options['SERVICE'],
Expand Down Expand Up @@ -429,11 +432,8 @@ def down(self, options):
Compose file
-t, --timeout TIMEOUT Specify a shutdown timeout in seconds.
(default: 10)
--env-file PATH Specify an alternate environment file
"""
environment_file = options.get('--env-file')
environment = Environment.from_env_file(self.project_dir, environment_file)
ignore_orphans = environment.get_boolean('COMPOSE_IGNORE_ORPHANS')
ignore_orphans = self.toplevel_environment.get_boolean('COMPOSE_IGNORE_ORPHANS')

if ignore_orphans and options['--remove-orphans']:
raise UserError("COMPOSE_IGNORE_ORPHANS and --remove-orphans cannot be combined.")
Expand Down Expand Up @@ -489,11 +489,8 @@ def exec_command(self, options):
-e, --env KEY=VAL Set environment variables (can be used multiple times,
not supported in API < 1.25)
-w, --workdir DIR Path to workdir directory for this command.
--env-file PATH Specify an alternate environment file
"""
environment_file = options.get('--env-file')
environment = Environment.from_env_file(self.project_dir, environment_file)
use_cli = not environment.get_boolean('COMPOSE_INTERACTIVE_NO_CLI')
use_cli = not self.toplevel_environment.get_boolean('COMPOSE_INTERACTIVE_NO_CLI')
index = int(options.get('--index'))
service = self.project.get_service(options['SERVICE'])
detach = options.get('--detach')
Expand All @@ -516,7 +513,7 @@ def exec_command(self, options):
if IS_WINDOWS_PLATFORM or use_cli and not detach:
sys.exit(call_docker(
build_exec_command(options, container.id, command),
self.toplevel_options, environment)
self.toplevel_options, self.toplevel_environment)
)

create_exec_options = {
Expand Down Expand Up @@ -890,7 +887,7 @@ def run(self, options):
container_options = build_one_off_container_options(options, detach, command)
run_one_off_container(
container_options, self.project, service, options,
self.toplevel_options, self.project_dir
self.toplevel_options, self.toplevel_environment
)

def scale(self, options):
Expand Down Expand Up @@ -1051,7 +1048,6 @@ def up(self, options):
container. Implies --abort-on-container-exit.
--scale SERVICE=NUM Scale SERVICE to NUM instances. Overrides the
`scale` setting in the Compose file if present.
--env-file PATH Specify an alternate environment file
"""
start_deps = not options['--no-deps']
always_recreate_deps = options['--always-recreate-deps']
Expand All @@ -1066,9 +1062,7 @@ def up(self, options):
if detached and (cascade_stop or exit_value_from):
raise UserError("--abort-on-container-exit and -d cannot be combined.")

environment_file = options.get('--env-file')
environment = Environment.from_env_file(self.project_dir, environment_file)
ignore_orphans = environment.get_boolean('COMPOSE_IGNORE_ORPHANS')
ignore_orphans = self.toplevel_environment.get_boolean('COMPOSE_IGNORE_ORPHANS')

if ignore_orphans and remove_orphans:
raise UserError("COMPOSE_IGNORE_ORPHANS and --remove-orphans cannot be combined.")
Expand Down Expand Up @@ -1331,7 +1325,7 @@ def build_one_off_container_options(options, detach, command):


def run_one_off_container(container_options, project, service, options, toplevel_options,
project_dir='.'):
toplevel_environment):
if not options['--no-deps']:
deps = service.get_dependency_names()
if deps:
Expand Down Expand Up @@ -1360,9 +1354,8 @@ def remove_container(force=False):
if options['--rm']:
project.client.remove_container(container.id, force=True, v=True)

environment_file = options.get('--env-file')
environment = Environment.from_env_file(project_dir, environment_file)
use_cli = not environment.get_boolean('COMPOSE_INTERACTIVE_NO_CLI')
use_cli = not toplevel_environment.get_boolean('COMPOSE_INTERACTIVE_NO_CLI')
Copy link
Contributor

@ulyssessouza ulyssessouza Jul 12, 2019

Choose a reason for hiding this comment

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

Why not using use_cli = not self.toplevel_environment.get_boolean('COMPOSE_INTERACTIVE_NO_CLI') here too?

Copy link
Author

@KlaasH KlaasH Jul 12, 2019

Choose a reason for hiding this comment

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

This is a stand-alone function, not a method of TopLevelCommand. Though it seems like it's only called in TopLevelCommand.run, so adding an argument to pass toplevel_environment in could work fine and remove this duplication.

Copy link
Author

@KlaasH KlaasH Jul 12, 2019

Choose a reason for hiding this comment

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

Added a commit to do this--replaced the project_dir argument (which was only used for this) with a toplevel_environment argument.

Copy link
Contributor

@ulyssessouza ulyssessouza Jul 15, 2019

Choose a reason for hiding this comment

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

Cool! I didn't have enough context in the browser based diff to see that.


signals.set_signal_handler_to_shutdown()
signals.set_signal_handler_to_hang_up()
try:
Expand All @@ -1371,7 +1364,7 @@ def remove_container(force=False):
service.connect_container_to_networks(container, use_network_aliases)
exit_code = call_docker(
get_docker_start_call(container_options, container.id),
toplevel_options, environment
toplevel_options, toplevel_environment
)
else:
operation = RunOperation(
Expand Down
2 changes: 1 addition & 1 deletion compose/config/environment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def split_env(env):
key = env
if re.search(r'\s', key):
raise ConfigurationError(
"environment variable name '{}' may not contains whitespace.".format(key)
"environment variable name '{}' may not contain whitespace.".format(key)
)
return key, value

Expand Down
5 changes: 5 additions & 0 deletions contrib/completion/bash/docker-compose
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,10 @@ _docker_compose_docker_compose() {
_filedir -d
return
;;
--env-file)
_filedir
return
;;
$(__docker_compose_to_extglob "$daemon_options_with_args") )
return
;;
Expand Down Expand Up @@ -612,6 +616,7 @@ _docker_compose() {
--tlsverify
"
local daemon_options_with_args="
--env-file
--file -f
--host -H
--project-directory
Expand Down
1 change: 1 addition & 0 deletions contrib/completion/fish/docker-compose.fish
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ end

complete -c docker-compose -s f -l file -r -d 'Specify an alternate compose file'
complete -c docker-compose -s p -l project-name -x -d 'Specify an alternate project name'
complete -c docker-compose -l env-file -r -d 'Specify an alternate environment file (default: .env)'
complete -c docker-compose -l verbose -d 'Show more output'
complete -c docker-compose -s H -l host -x -d 'Daemon socket to connect to'
complete -c docker-compose -l tls -d 'Use TLS; implied by --tlsverify'
Expand Down
2 changes: 2 additions & 0 deletions contrib/completion/zsh/_docker-compose
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,7 @@ _docker-compose() {
'(- :)'{-h,--help}'[Get help]' \
'*'{-f,--file}"[${file_description}]:file:_files -g '*.yml'" \
'(-p --project-name)'{-p,--project-name}'[Specify an alternate project name (default: directory name)]:project name:' \
'--env-file[Specify an alternate environment file (default: .env)]:env-file:_files' \
"--compatibility[If set, Compose will attempt to convert keys in v3 files to their non-Swarm equivalent]" \
'(- :)'{-v,--version}'[Print version and exit]' \
'--verbose[Show more output]' \
Expand All @@ -359,6 +360,7 @@ _docker-compose() {
local -a relevant_compose_flags relevant_compose_repeatable_flags relevant_docker_flags compose_options docker_options

relevant_compose_flags=(
"--env-file"
"--file" "-f"
"--host" "-H"
"--project-name" "-p"
Expand Down