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

Allow for multiple env-files #616

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 13 additions & 1 deletion python_on_whales/client_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ class ClientConfig:
compose_files: List[ValidPath] = field(default_factory=list)
compose_profiles: List[str] = field(default_factory=list)
compose_env_file: Optional[ValidPath] = None
compose_env_files: Iterable[ValidPath] = field(default_factory=list)
compose_project_name: Optional[str] = None
compose_project_directory: Optional[ValidPath] = None
compose_compatibility: Optional[bool] = None
Expand Down Expand Up @@ -159,7 +160,18 @@ def docker_compose_cmd(self) -> Command:
base_cmd = self.docker_cmd + ["compose"]
base_cmd.add_args_iterable_or_single("--file", self.compose_files)
base_cmd.add_args_iterable_or_single("--profile", self.compose_profiles)
base_cmd.add_simple_arg("--env-file", self.compose_env_file)
if self.compose_env_files:
if self.compose_env_file:
warnings.warn(
"You can't set both `compose_env_file` and `compose_env_files`. Files used in `compose_env_files` will be used."
)
base_cmd.add_args_iterable("--env-file", self.compose_env_files)
elif self.compose_env_file:
warnings.warn(
"`compose_env_file` is deprecated. Use `compose_env_files` instead."
)
base_cmd.add_simple_arg("--env-file", self.compose_env_file)

base_cmd.add_simple_arg("--project-name", self.compose_project_name)
base_cmd.add_simple_arg("--project-directory", self.compose_project_directory)
base_cmd.add_flag("--compatibility", self.compose_compatibility)
Expand Down
2 changes: 2 additions & 0 deletions python_on_whales/docker_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ def __init__(
compose_files: List[ValidPath] = [],
compose_profiles: List[str] = [],
compose_env_file: Optional[ValidPath] = None,
compose_env_files: List[ValidPath] = [],
compose_project_name: Optional[str] = None,
compose_project_directory: Optional[ValidPath] = None,
compose_compatibility: Optional[bool] = None,
Expand Down Expand Up @@ -209,6 +210,7 @@ def __init__(
compose_files=compose_files,
compose_profiles=compose_profiles,
compose_env_file=compose_env_file,
compose_env_files=compose_env_files,
compose_project_name=compose_project_name,
compose_project_directory=compose_project_directory,
compose_compatibility=compose_compatibility,
Expand Down
16 changes: 11 additions & 5 deletions tests/python_on_whales/components/dummy_compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ services:
context: my_service_build
image: some_random_image
ports:
- "5000:5000"
- "5000:5000"
volumes:
- /tmp:/tmp
- /tmp:/tmp

environment:
- DATADOG_HOST=something
- DATADOG_HOST=something
busybox:
image: busybox:latest
command: sleep infinity
Expand All @@ -24,8 +24,8 @@ services:
image: alpine:latest
command: sleep infinity
environment:
- DD_API_KEY=__your_datadog_api_key_here__
- POSTGRES_HOST_AUTH_METHOD=trust
- DD_API_KEY=__your_datadog_api_key_here__
einarwar marked this conversation as resolved.
Show resolved Hide resolved
- POSTGRES_HOST_AUTH_METHOD=trust
dodo:
image: busybox:latest
entrypoint: /bin/sh
Expand All @@ -39,3 +39,9 @@ services:
command: sleep infinity
profiles:
- my_test_profile2
service_using_env_variables:
image: busybox:latest
command: sleep infinity
environment:
- A=${A:-0}
- B=${B:-0}
143 changes: 141 additions & 2 deletions tests/python_on_whales/test_client_config.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import json
from contextlib import contextmanager
from pathlib import Path
from typing import Iterator, Sequence, Tuple

import pytest

from python_on_whales import docker
from python_on_whales.client_config import ParsingError
from python_on_whales import DockerClient, docker
from python_on_whales.client_config import ClientConfig, ParsingError
from python_on_whales.utils import PROJECT_ROOT

fake_json_message = {
"CreatedAt": "2020-10-08T18:32:55Z",
Expand Down Expand Up @@ -36,3 +39,139 @@ def test_pretty_exception_message_and_report(mocker):
raise IndexError

assert Path(word).read_text() == json.dumps(fake_json_message, indent=2)


def test_compose_env_file():
"""Test that the deprecated `compose_env_file` gives a warning, and adds the `--env-file` argument to the compose command"""
with pytest.warns(UserWarning):
example_env_file_name = "example.env"
client_config = ClientConfig(compose_env_file=example_env_file_name)
assert client_config.docker_compose_cmd.count("--env-file") == 1
# Since we are operating of a list of commands, we first find the index of the `--env-file` argument, then we check that the next argument is the file name.
index = client_config.docker_compose_cmd.index("--env-file")
assert client_config.docker_compose_cmd[index + 1] == example_env_file_name


@pytest.mark.parametrize(
"compose_env_files",
[
["example1.env", "example2.env"],
["example1.env"],
],
ids=["multiple_files", "single_file"],
)
def test_compose_env_files(compose_env_files: Sequence[str]):
"""Test that using only `compose_env_files` adds all the files to the command line arguments"""
client_config = ClientConfig(compose_env_files=compose_env_files)
# Since we are operating of a list of commands, we first find the indices of the `--env-file` argument, then we check that the next argument is the file name.
indices = [
index
for index, item in enumerate(client_config.docker_compose_cmd)
if item == "--env-file"
]
assert len(indices) == len(compose_env_files)
for index, env_file in zip(indices, compose_env_files):
assert client_config.docker_compose_cmd[index + 1] == env_file


def test_compose_env_files_and_env_file():

Choose a reason for hiding this comment

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

The tests are good, keep them :) I'd also like a test which actually runs the docker compose command and checks that the env files are loaded correctly. For example with docker.compose.config(). If for example, you made a typo in --env-file in the unit test and the main implementation, your tests can't currently detect it. Running the true command in test would detect such an error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added some more tests now that actually run the command by using the equivalent of docker compose run. Some temporary env-files are made, and the containers are then inspected to verify that the actual environment variables set in the container are as expected. Pretty verbose, and I am sure it can be done simpler, but let me know if this is what you wanted :)

Choose a reason for hiding this comment

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

Rather than running the containers, using https://gabrieldemarmiesse.github.io/python-on-whales/sub-commands/compose/#python_on_whales.components.compose.cli_wrapper.ComposeCLI.config would have made the test simpler no? just switch docker.compose.run for docker.compose.config()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aha, yeah that is a lot simpler. Updated now :)

"""Test that using both `compose_env_files` and `compose_env_file` gives a warning, and only uses `compose_env_files`"""
env_files_input = "example1.env"
env_file_input = "example2.env"

with pytest.warns(UserWarning):
client_config = ClientConfig(
compose_env_files=[env_files_input], compose_env_file=env_file_input
)
assert client_config.docker_compose_cmd.count("--env-file") == len(
[env_files_input]
)
# Since we are operating of a list of commands, we first find the index of the `--env-file` argument, then we check that the next argument is the file name.
index = client_config.docker_compose_cmd.index("--env-file")
assert client_config.docker_compose_cmd[index + 1] == env_files_input


@contextmanager
def _create_temp_env_files() -> Iterator[Tuple[Path, Path]]:
"""Creates two temporary env files and yields their paths. The files are deleted after the context manager is exited."""
path = PROJECT_ROOT / "tests/python_on_whales/components"
env_file_1 = path / "example1.env"
with env_file_1.open("w") as f:
f.write("A=1\n")
f.write("B=1\n")
env_file_2 = path / "example2.env"
with env_file_2.open("w") as f:
f.write("B=2\n")
yield env_file_1, env_file_2
env_file_1.unlink()
env_file_2.unlink()


def test_run_compose_command_with_env_file():
"""Test that checks the configuration for the compose stack and checks that the env files are loaded correctly

The docker compose service, `service_using_env_variables`, is defined in `dummy_compose.yml` and uses the env variables `A` and `B`.
If no env files are provided, the values of `A` and `B` are set to a default value of `0` in the service.
We check if the values of `A` and `B` are set to the values in the env files.

The cases tested are:
* Single env file using the old `compose_env_file` argument
* Single env file using the new `compose_env_files` argument
* Multiple env files using the new `compose_env_files` argument
* Later env files should override the values of earlier env files
* The order of the env files should matter
"""
with _create_temp_env_files() as (env_file_1, env_file_2):
COMPOSE_FILE = (
PROJECT_ROOT / "tests/python_on_whales/components/dummy_compose.yml"
)
SERVICE = "service_using_env_variables"

# Test with no env-file to check the default values, should be `0`, as is default in the service definition.
client = DockerClient(compose_files=[COMPOSE_FILE])
config = client.compose.config()
environment = config.services[SERVICE].environment
assert "A" in environment and environment["A"] == "0"
assert "B" in environment and environment["B"] == "0"

# Test with single env-file using the old `compose_env_file` argument
client = DockerClient(
compose_files=[COMPOSE_FILE],
compose_env_file=env_file_1,
)
config = client.compose.config()
environment = config.services[SERVICE].environment
assert "A" in environment and environment["A"] == "1"
assert "B" in environment and environment["B"] == "1"

# Test with single env-file using the new `compose_env_files` argument
client = DockerClient(
compose_files=[COMPOSE_FILE],
compose_env_files=[env_file_1],
)
config = client.compose.config()
environment = config.services[SERVICE].environment
assert "A" in environment and environment["A"] == "1"
assert "B" in environment and environment["B"] == "1"

# Test with multiple env-files using the new `compose_env_files` argument.
# Since `env_file2` is loaded after `env_file1`, the value of `B` should be overridden by `env_file_2`.
client = DockerClient(
compose_files=[COMPOSE_FILE],
compose_env_files=[env_file_1, env_file_2],
)
config = client.compose.config()
environment = config.services[SERVICE].environment
assert "A" in environment and environment["A"] == "1"
assert "B" in environment and environment["B"] == "2"

# Test with multiple env-files using the new `compose_env_files` argument.
# Since `env_file1` is loaded after `env_file2`, the value of `B` should be overridden by `env_file_1`.
client = DockerClient(
compose_files=[COMPOSE_FILE],
compose_env_files=[env_file_2, env_file_1],
)
config = client.compose.config()
environment = config.services[SERVICE].environment
assert "A" in environment and environment["A"] == "1"
assert "B" in environment and environment["B"] == "1"
Loading