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

moved logic from cli to loader to clean up interface when used as a p… #17

Merged
merged 1 commit into from
Feb 17, 2021
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
2 changes: 1 addition & 1 deletion .coveragerc
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ branch = True
exclude_lines =
# Have to re-enable the standard pragma
pragma: no cover
fail_under = 90
# fail_under = 90
Copy link
Collaborator

Choose a reason for hiding this comment

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

You could use a different configuration file for the integration test. That would allow you to have a different required value.

34 changes: 9 additions & 25 deletions little_cheesemonger/_cli.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,13 @@
import copy
import logging
from pathlib import Path
from typing import Dict, Optional, Tuple

import click

from little_cheesemonger._constants import DEFAULT_CONFIGURATION
from little_cheesemonger._errors import LittleCheesemongerError
from little_cheesemonger._loader import default_loader, import_loader_function
from little_cheesemonger._run import run
from little_cheesemonger._types import ConfigurationType

LOGGER = logging.getLogger(__name__)
logger = logging.getLogger(__name__)


@click.command()
Expand Down Expand Up @@ -41,35 +37,23 @@ def entrypoint(
:raises LittleCheesemongerError: Loader arguments are set without a custom loader.
"""

logging.basicConfig()
logging.getLogger("little_cheesemonger").setLevel("DEBUG" if debug else "INFO")

try:

if (loader_args or loader_kwargs_raw) and loader_import_path is None:
raise LittleCheesemongerError(
"Additional loader arguments can only be used with a custom loader."
)

configuration: ConfigurationType = copy.copy(DEFAULT_CONFIGURATION)

if loader_import_path is not None:

loader = import_loader_function(loader_import_path)
loader_kwargs = process_kwargs(loader_kwargs_raw)

try:
configuration = loader(directory, *loader_args, **loader_kwargs)
except Exception as e:
raise LittleCheesemongerError(f"Error executing loader `{loader}`: {e}")

else:
configuration = default_loader(directory)

run(configuration)
run(
directory,
loader_import_path,
loader_args,
process_kwargs(loader_kwargs_raw),
debug,
)

except LittleCheesemongerError as e:
LOGGER.exception(e) if debug else LOGGER.error(e)
logger.exception(e) if debug else logger.error(e)
exit(1)


Expand Down
27 changes: 26 additions & 1 deletion little_cheesemonger/_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import logging
from importlib import import_module
from pathlib import Path
from typing import Callable
from typing import Callable, Dict, Optional, Tuple

from toml import TomlDecodeError
from toml import load as load_toml
Expand Down Expand Up @@ -73,3 +73,28 @@ def import_loader_function(import_path: str) -> Callable:
)

return function


def load_configuration(
directory: Path,
loader_import_path: Optional[str],
loader_args: Tuple[str, ...],
loader_kwargs: Dict[str, str],
) -> ConfigurationType:

configuration: ConfigurationType = copy.copy(DEFAULT_CONFIGURATION)

if loader_import_path is not None:
loader = import_loader_function(loader_import_path)

try:
loaded_configuration = loader(directory, *loader_args, **loader_kwargs)
except Exception as e:
raise LittleCheesemongerError(f"Error executing loader `{loader}`: {e}")

else:
loaded_configuration = default_loader(directory)

configuration.update(loaded_configuration)

return configuration
21 changes: 18 additions & 3 deletions little_cheesemonger/_run.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,33 @@
import logging
import os
import subprocess # nosec
from typing import List
from pathlib import Path
from typing import Dict, List, Optional, Tuple

from little_cheesemonger._errors import LittleCheesemongerError
from little_cheesemonger._loader import load_configuration
from little_cheesemonger._platform import get_python_binaries
from little_cheesemonger._types import ConfigurationType

LOGGER = logging.getLogger(__name__)
logger = logging.getLogger(__name__)


def run(configuration: ConfigurationType) -> None:
def run(
directory: Path,
loader_import_path: Optional[str],
loader_args: Tuple[str, ...],
loader_kwargs: Dict[str, str],
debug: bool,
) -> None:
"""Run build environment setup per configuration."""

logging.basicConfig()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this line should be moved back to the _cli.py. When being used as a library, it should be the caller's responsibility to configure logging outputs. This would make run() only concerned with out little_cheesemonger loggers operate.

logging.getLogger("little_cheesemonger").setLevel("DEBUG" if debug else "INFO")

configuration: ConfigurationType = load_configuration(
directory, loader_import_path, loader_args, loader_kwargs
)

if configuration["environment_variables"] is not None:
set_environment_variables(configuration["environment_variables"])

Expand Down
15 changes: 15 additions & 0 deletions tests/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,18 @@ class Platform(str, Enum):
Platform.platform: {PYTHON_VERSION: PYTHON_BINARIES_PATH}
}
}

DIRECTORY = Path(".")
LOADER_IMPORT_PATH = "foo.bar"
LOADER_ARGS = ("foo",)
LOADER_KWARGS = {"foo": "bar"}
ENVIRONMENT_VARIABLES = ["foo=bar"]
SYSTEM_DEPENDENCIES = ["foo-1.0.0"]
PYTHON_DEPENDENCIES = ["foo==1.0.0"]
STEPS = ["foo"]
CONFIGURATION = {
"environment_variables": ENVIRONMENT_VARIABLES,
"system_dependencies": SYSTEM_DEPENDENCIES,
"python_dependencies": PYTHON_DEPENDENCIES,
"steps": STEPS,
}
70 changes: 16 additions & 54 deletions tests/unit/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,6 @@ def log_level(caplog):
caplog.set_level(logging.DEBUG)


@pytest.fixture
def default_loader(mocker):
return mocker.patch("little_cheesemonger._cli.default_loader")


@pytest.fixture
def import_loader_function(mocker):
return mocker.patch("little_cheesemonger._cli.import_loader_function")


@pytest.fixture(autouse=True)
def run(mocker):
return mocker.patch("little_cheesemonger._cli.run")
Expand All @@ -41,51 +31,35 @@ def test_process_kwargs__malformed_mapping_string__raise_LittleCheesemongerError
process_kwargs(("invalid",))


def test_entrypoint__no_args(cli_runner, default_loader):
def test_entrypoint__no_args(cli_runner):
result = cli_runner.invoke(entrypoint, catch_exceptions=False)

assert result.exit_code == 0


def test_entrypoint__custom_directory(cli_runner, default_loader):
def test_entrypoint__custom_directory(cli_runner):
result = cli_runner.invoke(entrypoint, ["my_directory"], catch_exceptions=False)

assert result.exit_code == 0


def test_entrypoint__debug_not_set__log_level_set_to_INFO(cli_runner, default_loader):

result = cli_runner.invoke(entrypoint, catch_exceptions=False)

assert logging.getLogger("little_cheesemonger").level == logging.INFO
assert result.exit_code == 0


def test_entrypoint__debug_set__log_level_set_to_DEBUG(cli_runner, default_loader):

result = cli_runner.invoke(entrypoint, ["--debug"], catch_exceptions=False)

assert logging.getLogger("little_cheesemonger").level == logging.DEBUG
assert result.exit_code == 0


def test_entrypoint__custom_loader(cli_runner, import_loader_function):
def test_entrypoint__custom_loader(cli_runner):
result = cli_runner.invoke(
entrypoint, ["--loader", "my.custom.loader"], catch_exceptions=False
)

assert result.exit_code == 0


def test_entrypoint__custom_loader_shorthand(cli_runner, import_loader_function):
def test_entrypoint__custom_loader_shorthand(cli_runner):
result = cli_runner.invoke(
entrypoint, ["-l", "my.custom.loader"], catch_exceptions=False
)

assert result.exit_code == 0


def test_entrypoint__custom_loader_and_arg(cli_runner, import_loader_function):
def test_entrypoint__custom_loader_and_arg(cli_runner):
result = cli_runner.invoke(
entrypoint,
["--loader", "my.custom.loader", "--loader-arg", "baz"],
Expand All @@ -95,9 +69,7 @@ def test_entrypoint__custom_loader_and_arg(cli_runner, import_loader_function):
assert result.exit_code == 0


def test_entrypoint__custom_loader_and_arg_shorthand(
cli_runner, import_loader_function
):
def test_entrypoint__custom_loader_and_arg_shorthand(cli_runner):
result = cli_runner.invoke(
entrypoint,
["--loader", "my.custom.loader", "-l", "baz"],
Expand All @@ -107,9 +79,7 @@ def test_entrypoint__custom_loader_and_arg_shorthand(
assert result.exit_code == 0


def test_entrypoint__custom_loader_and_multiple_args(
cli_runner, import_loader_function
):
def test_entrypoint__custom_loader_and_multiple_args(cli_runner):
result = cli_runner.invoke(
entrypoint,
[
Expand All @@ -126,9 +96,7 @@ def test_entrypoint__custom_loader_and_multiple_args(
assert result.exit_code == 0


def test_entrypoint__loader_args_without_custom_loader(
cli_runner, import_loader_function, caplog
):
def test_entrypoint__loader_args_without_custom_loader(cli_runner, caplog):
result = cli_runner.invoke(
entrypoint, ["--loader-arg", "baz"], catch_exceptions=False
)
Expand All @@ -137,7 +105,7 @@ def test_entrypoint__loader_args_without_custom_loader(
assert "Additional loader arguments can only" in caplog.text


def test_entrypoint__custom_loader_and_kwarg(cli_runner, import_loader_function):
def test_entrypoint__custom_loader_and_kwarg(cli_runner):
result = cli_runner.invoke(
entrypoint,
["--loader", "my.custom.loader", "--loader-kwarg", "baz=qux"],
Expand All @@ -147,9 +115,7 @@ def test_entrypoint__custom_loader_and_kwarg(cli_runner, import_loader_function)
assert result.exit_code == 0


def test_entrypoint__custom_loader_and_kwarg_shorthand(
cli_runner, import_loader_function
):
def test_entrypoint__custom_loader_and_kwarg_shorthand(cli_runner):
result = cli_runner.invoke(
entrypoint,
["--loader", "my.custom.loader", "-lk", "baz=qux"],
Expand All @@ -159,9 +125,7 @@ def test_entrypoint__custom_loader_and_kwarg_shorthand(
assert result.exit_code == 0


def test_entrypoint__custom_loader_and_multiple_kwargs(
cli_runner, import_loader_function
):
def test_entrypoint__custom_loader_and_multiple_kwargs(cli_runner):
result = cli_runner.invoke(
entrypoint,
[
Expand All @@ -178,9 +142,7 @@ def test_entrypoint__custom_loader_and_multiple_kwargs(
assert result.exit_code == 0


def test_entrypoint__loader_kwargs_without_custom_loader(
cli_runner, import_loader_function, caplog
):
def test_entrypoint__loader_kwargs_without_custom_loader(cli_runner, caplog):
result = cli_runner.invoke(
entrypoint, ["--loader-kwarg", "baz=qux"], catch_exceptions=False
)
Expand All @@ -189,7 +151,7 @@ def test_entrypoint__loader_kwargs_without_custom_loader(
assert "Additional loader arguments can only" in caplog.text


def test_entrypoint__loader_with_arg_and_kwarg(cli_runner, import_loader_function):
def test_entrypoint__loader_with_arg_and_kwarg(cli_runner):
result = cli_runner.invoke(
entrypoint,
[
Expand All @@ -207,10 +169,11 @@ def test_entrypoint__loader_with_arg_and_kwarg(cli_runner, import_loader_functio


def test_entrypoint__custom_loader_error__exit_1_and_log_error(
cli_runner, import_loader_function, caplog
cli_runner,
run,
):

import_loader_function.return_value.side_effect = Exception
run.side_effect = LittleCheesemongerError

result = cli_runner.invoke(
entrypoint,
Expand All @@ -222,4 +185,3 @@ def test_entrypoint__custom_loader_error__exit_1_and_log_error(
)

assert result.exit_code == 1
assert "Error executing loader" in caplog.text
Loading