From 803a91c0723533f62148528a81f9d0411b57438e Mon Sep 17 00:00:00 2001 From: Dmitrii Agibov Date: Mon, 20 Feb 2023 15:42:33 +0000 Subject: MLIA-813 Change default output directory - Use directory mlia-output as output directory for MLIA - If parameter --output-dir provided then place directory mlia-output under specified path or otherwise create it in the current working directory Change-Id: I298088c4aa8dbe9f35dee69ecb9ff6e9ea3cac0a --- src/mlia/cli/main.py | 16 +++++++---- src/mlia/cli/options.py | 9 +++--- src/mlia/core/context.py | 38 ++++++++++++------------- src/mlia/utils/filesystem.py | 13 +++++++++ tests/test_cli_main.py | 2 +- tests/test_core_context.py | 63 +++++++++++++++++------------------------- tests/test_utils_filesystem.py | 38 +++++++++++++++++++++++++ tests/utils/common.py | 12 +++++++- 8 files changed, 121 insertions(+), 70 deletions(-) diff --git a/src/mlia/cli/main.py b/src/mlia/cli/main.py index cc97494..88258d5 100644 --- a/src/mlia/cli/main.py +++ b/src/mlia/cli/main.py @@ -160,12 +160,16 @@ def setup_context( args: argparse.Namespace, context_var_name: str = "ctx" ) -> tuple[ExecutionContext, dict]: """Set up context and resolve function parameters.""" - ctx = ExecutionContext( - verbose="debug" in args and args.debug, - action_resolver=CLIActionResolver(vars(args)), - output_format=get_output_format(args), - output_dir=args.output_dir if "output_dir" in args else None, - ) + try: + ctx = ExecutionContext( + verbose="debug" in args and args.debug, + action_resolver=CLIActionResolver(vars(args)), + output_format=get_output_format(args), + output_dir=args.output_dir if "output_dir" in args else None, + ) + except Exception as err: # pylint: disable=broad-except + print(f"Error: {err}", file=sys.stderr) + sys.exit(1) setup_logging(ctx.logs_path, ctx.verbose, ctx.output_format) diff --git a/src/mlia/cli/options.py b/src/mlia/cli/options.py index d40aa88..fe177eb 100644 --- a/src/mlia/cli/options.py +++ b/src/mlia/cli/options.py @@ -231,10 +231,11 @@ def add_output_directory(parser: argparse.ArgumentParser) -> None: parser.add_argument( "--output-dir", type=Path, - help="Path to the directory where MLIA " - "stores artifacts, e.g. logs, target profiles and model files. " - "If not specified then MLIA will use " - "directory 'mlia-output' in the current working directory instead.", + help="Path to the directory where MLIA will create " + "output directory 'mlia-output' " + "for storing artifacts, e.g. logs, target profiles and model files. " + "If not specified then 'mlia-output' directory will be created " + "in the current working directory.", ) diff --git a/src/mlia/core/context.py b/src/mlia/core/context.py index e8c8a2c..d3fa606 100644 --- a/src/mlia/core/context.py +++ b/src/mlia/core/context.py @@ -12,18 +12,20 @@ from __future__ import annotations import logging from abc import ABC from abc import abstractmethod -from datetime import datetime from pathlib import Path from typing import Any from typing import Mapping from mlia.core.common import AdviceCategory +from mlia.core.errors import ConfigurationError from mlia.core.events import DefaultEventPublisher from mlia.core.events import EventHandler from mlia.core.events import EventPublisher from mlia.core.helpers import ActionResolver from mlia.core.helpers import APIActionResolver from mlia.core.typing import OutputFormat +from mlia.utils.filesystem import recreate_directory +from mlia.utils.filesystem import USER_ONLY_PERM_MASK logger = logging.getLogger(__name__) @@ -97,18 +99,6 @@ class Context(ABC): self.event_publisher.register_event_handlers(self.event_handlers) -def create_output_dir_with_timestamp() -> Path: - """Generate output directory for the MLIA.""" - base_dir = Path.cwd() / "mlia-output" - base_dir.mkdir(exist_ok=True) - - timestamp = datetime.now().isoformat().replace(":", "") - output_dir = base_dir / f"mlia-output-{timestamp}" - output_dir.mkdir() - - return output_dir - - class ExecutionContext(Context): """Execution context.""" @@ -250,10 +240,18 @@ class ExecutionContext(Context): def _init_output_directory(self, output_dir: str | Path | None) -> None: """Init output directory for the execution.""" - if output_dir: - output_dir_path = Path(output_dir) - output_dir_path.mkdir(exist_ok=True) - else: - output_dir_path = create_output_dir_with_timestamp() - - self._output_dir_path = output_dir_path + try: + if output_dir: + output_dir_location = Path(output_dir) + output_dir_location.mkdir(exist_ok=True, mode=USER_ONLY_PERM_MASK) + else: + output_dir_location = Path.cwd() + + output_dir_path = output_dir_location / "mlia-output" + recreate_directory(output_dir_path) + + self._output_dir_path = output_dir_path + except OSError as err: + raise ConfigurationError( + f"Unable to create output directory: {err}." + ) from err diff --git a/src/mlia/utils/filesystem.py b/src/mlia/utils/filesystem.py index f92629b..f8e8962 100644 --- a/src/mlia/utils/filesystem.py +++ b/src/mlia/utils/filesystem.py @@ -14,6 +14,8 @@ from tempfile import TemporaryDirectory from typing import Generator from typing import Iterable +USER_ONLY_PERM_MASK = 0o700 + def get_mlia_resources() -> Path: """Get the path to the resources directory.""" @@ -96,6 +98,17 @@ def copy_all(*paths: Path, dest: Path) -> None: shutil.copytree(path, dest, dirs_exist_ok=True) +def recreate_directory(dir_path: Path, mode: int = USER_ONLY_PERM_MASK) -> None: + """Recreate directory.""" + if dir_path.exists(): + if not dir_path.is_dir(): + raise ValueError(f"Path {dir_path} is not a directory.") + + shutil.rmtree(dir_path) + + dir_path.mkdir(exist_ok=True, mode=mode) + + @contextmanager def working_directory( working_dir: Path, create_dir: bool = False diff --git a/tests/test_cli_main.py b/tests/test_cli_main.py index c129c4b..2f89268 100644 --- a/tests/test_cli_main.py +++ b/tests/test_cli_main.py @@ -273,7 +273,7 @@ def test_passing_output_directory_parameter( main(["sample_command", "--output-dir", output_dir.as_posix()]) assert passed_context is not None - assert passed_context.output_dir == output_dir + assert passed_context.output_dir == output_dir / "mlia-output" @pytest.mark.parametrize( diff --git a/tests/test_core_context.py b/tests/test_core_context.py index 09b34e9..0810ad0 100644 --- a/tests/test_core_context.py +++ b/tests/test_core_context.py @@ -3,16 +3,16 @@ """Tests for the module context.""" from __future__ import annotations -import re from pathlib import Path import pytest from mlia.core.common import AdviceCategory -from mlia.core.context import create_output_dir_with_timestamp from mlia.core.context import ExecutionContext from mlia.core.events import DefaultEventPublisher +from mlia.utils.filesystem import USER_ONLY_PERM_MASK from mlia.utils.filesystem import working_directory +from tests.utils.common import check_expected_permissions @pytest.mark.parametrize( @@ -46,7 +46,7 @@ def test_execution_context_category_enabled( assert ctx.category_enabled(category) -def test_execution_context(tmpdir: str) -> None: +def test_execution_context(tmp_path: Path) -> None: """Test execution context.""" publisher = DefaultEventPublisher() category = {AdviceCategory.COMPATIBILITY} @@ -54,7 +54,7 @@ def test_execution_context(tmpdir: str) -> None: context = ExecutionContext( advice_category=category, config_parameters={"param": "value"}, - output_dir=tmpdir, + output_dir=tmp_path / "output", event_handlers=[], event_publisher=publisher, verbose=True, @@ -63,18 +63,24 @@ def test_execution_context(tmpdir: str) -> None: output_format="json", ) + output_dir = context.output_dir + assert output_dir == tmp_path.joinpath("output", "mlia-output") + assert output_dir.is_dir() + check_expected_permissions(output_dir, USER_ONLY_PERM_MASK) + check_expected_permissions(tmp_path.joinpath("output"), USER_ONLY_PERM_MASK) + assert context.advice_category == category assert context.config_parameters == {"param": "value"} assert context.event_handlers == [] assert context.event_publisher == publisher - assert context.logs_path == Path(tmpdir) / "logs_directory" - expected_model_path = Path(tmpdir) / "models_directory/sample.model" + assert context.logs_path == output_dir / "logs_directory" + expected_model_path = output_dir / "models_directory/sample.model" assert context.get_model_path("sample.model") == expected_model_path assert context.verbose is True assert context.output_format == "json" assert str(context) == ( f"ExecutionContext: " - f"output_dir={tmpdir}, " + f"output_dir={output_dir}, " "advice_category={'COMPATIBILITY'}, " "config_parameters={'param': 'value'}, " "verbose=True, " @@ -82,53 +88,34 @@ def test_execution_context(tmpdir: str) -> None: ) -def test_execution_context_with_default_params(tmpdir: str) -> None: +def test_execution_context_with_default_params(tmp_path: Path) -> None: """Test execution context with the default parameters.""" - context_with_default_params = ExecutionContext(output_dir=tmpdir) + working_dir = tmp_path / "sample" + with working_directory(working_dir, create_dir=True): + context_with_default_params = ExecutionContext() + assert context_with_default_params.advice_category == {AdviceCategory.COMPATIBILITY} assert context_with_default_params.config_parameters is None assert context_with_default_params.event_handlers is None assert isinstance( context_with_default_params.event_publisher, DefaultEventPublisher ) - assert context_with_default_params.logs_path == Path(tmpdir) / "logs" + + output_dir = context_with_default_params.output_dir + assert output_dir == working_dir.joinpath("mlia-output") + + assert context_with_default_params.logs_path == output_dir / "logs" default_model_path = context_with_default_params.get_model_path("sample.model") - expected_default_model_path = Path(tmpdir) / "models/sample.model" + expected_default_model_path = output_dir / "models/sample.model" assert default_model_path == expected_default_model_path assert context_with_default_params.output_format == "plain_text" expected_str = ( - f"ExecutionContext: output_dir={tmpdir}, " + f"ExecutionContext: output_dir={output_dir}, " "advice_category={'COMPATIBILITY'}, " "config_parameters=None, " "verbose=False, " "output_format=plain_text" ) assert str(context_with_default_params) == expected_str - - -def test_create_output_dir_with_timestamp(tmp_path: Path) -> None: - """Test function generate_output_dir_with_timestamp.""" - working_dir = tmp_path / "sample" - with working_directory(working_dir, create_dir=True): - create_output_dir_with_timestamp() - - working_dir_content = list(working_dir.iterdir()) - assert len(working_dir_content) == 1 - - parent_dir = working_dir_content[0] - assert parent_dir.is_dir() - assert parent_dir.name == "mlia-output" - - parent_dir_content = list(parent_dir.iterdir()) - assert len(parent_dir_content) == 1 - - output_dir = parent_dir_content[0] - assert output_dir.is_dir() - - pattern = re.compile(r"mlia-output-\d{4}-\d{2}-\d{2}T\d+[.]\d+") - assert pattern.match(output_dir.name) - - output_dir_content = list(output_dir.iterdir()) - assert not output_dir_content diff --git a/tests/test_utils_filesystem.py b/tests/test_utils_filesystem.py index d0a6e6f..654f5c8 100644 --- a/tests/test_utils_filesystem.py +++ b/tests/test_utils_filesystem.py @@ -12,10 +12,13 @@ from mlia.utils.filesystem import copy_all from mlia.utils.filesystem import get_mlia_resources from mlia.utils.filesystem import get_mlia_target_profiles_dir from mlia.utils.filesystem import get_vela_config +from mlia.utils.filesystem import recreate_directory from mlia.utils.filesystem import sha256 from mlia.utils.filesystem import temp_directory from mlia.utils.filesystem import temp_file +from mlia.utils.filesystem import USER_ONLY_PERM_MASK from mlia.utils.filesystem import working_directory +from tests.utils.common import check_expected_permissions def test_get_mlia_resources() -> None: @@ -135,3 +138,38 @@ def test_working_directory_context_manager( assert Path.cwd() == current_working_dir assert Path.cwd() == prev_wd + + +def test_recreate_directory(tmp_path: Path) -> None: + """Test function recreate_directory.""" + sample_dir = tmp_path / "sample" + sample_dir.mkdir() + + test_dir1 = sample_dir / "test_dir1" + test_dir1.mkdir() + + test_file1 = test_dir1 / "sample_file1.txt" + test_file1.touch() + + test_dir2 = sample_dir / "test_dir2" + recreate_directory(test_dir2) + + assert test_dir1.is_dir() + assert test_file1.is_file() + assert test_dir2.is_dir() + check_expected_permissions(test_dir2, USER_ONLY_PERM_MASK) + + recreate_directory(test_dir1) + assert test_dir2.is_dir() + assert not test_file1.exists() + assert test_dir1.is_dir() + check_expected_permissions(test_dir1, USER_ONLY_PERM_MASK) + + +def test_recreate_directory_wrong_path(tmp_path: Path) -> None: + """Test that function should fail if provided path is not a directory.""" + sample_file = tmp_path / "sample_file.txt" + sample_file.touch() + + with pytest.raises(ValueError, match=rf"Path {sample_file} is not a directory."): + recreate_directory(sample_file) diff --git a/tests/utils/common.py b/tests/utils/common.py index 616a407..c29b47c 100644 --- a/tests/utils/common.py +++ b/tests/utils/common.py @@ -1,8 +1,10 @@ -# SPDX-FileCopyrightText: Copyright 2022, Arm Limited and/or its affiliates. +# SPDX-FileCopyrightText: Copyright 2022-2023, Arm Limited and/or its affiliates. # SPDX-License-Identifier: Apache-2.0 """Common test utils module.""" from __future__ import annotations +from pathlib import Path + import numpy as np import tensorflow as tf @@ -30,3 +32,11 @@ def train_model(model: tf.keras.Model) -> None: x_train, y_train = get_dataset() model.fit(x_train, y_train, epochs=num_epochs) + + +def check_expected_permissions(path: Path, expected_permissions_mask: int) -> None: + """Check expected permissions for the provided path.""" + path_mode = path.stat().st_mode + permissions_mask = path_mode & 0o777 + + assert permissions_mask == expected_permissions_mask -- cgit v1.2.1