From fb7f432c98dfdfb49c2b9d4a68248191884edd30 Mon Sep 17 00:00:00 2001 From: jkanem Date: Mon, 7 Feb 2022 18:20:22 -0500 Subject: [PATCH] Fix missing config keys (#92) * If a user's configuration file is missing a parameter, the default parameter is used. * Updated CHANGELOG and VERSION. * Instead of setting configuration data to _DEFAULT_CONFIG, it is set to a copy so that further modification doesn't modify _DEFAULT_CONFIG. * Added tests for 2 configuration file operations. * Updated date in CHANGELOG. * Added a sample covalent.conf file for configuration tests. * Mocked the config file location for configuration unit tests. * Attempting again to mock configuration file location in config tests. * Attempting again to mock configuration file location in config tests. * Attempting again to mock configuration file location in config tests in pipeline. * Fixed a path error in config_test.py. * Updated VERSION * Change global varialbe to all-caps in config_test.py. * Moved config_test.py (and its test files) to covalent_tests/shared_files/. --- CHANGELOG.md | 6 + VERSION | 2 +- covalent/_shared_files/config.py | 30 ++++- .../choosing_conda_environments.ipynb | 6 +- .../shared_files/config_test.py | 127 ++++++++++++++++++ .../test_files/covalent/covalent.conf | 22 +++ tests/functional_tests/covalent.conf | 22 +++ 7 files changed, 208 insertions(+), 7 deletions(-) create mode 100644 tests/covalent_tests/shared_files/config_test.py create mode 100644 tests/covalent_tests/shared_files/test_files/covalent/covalent.conf create mode 100644 tests/functional_tests/covalent.conf diff --git a/CHANGELOG.md b/CHANGELOG.md index 04b976661..fc17638cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.24.8] - 2022-02-07 + +### Fixed + +- If a user's configuration file does not have a needed parameter, the default parameter (defined in _shared_files/defaults.py) is used. + ## [0.24.7] - 2022-02-07 ### Added diff --git a/VERSION b/VERSION index c9731ff41..72c1f4101 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.24.7 +0.24.8 \ No newline at end of file diff --git a/covalent/_shared_files/config.py b/covalent/_shared_files/config.py index 742b926ac..81f5c1e42 100644 --- a/covalent/_shared_files/config.py +++ b/covalent/_shared_files/config.py @@ -18,6 +18,7 @@ # # Relief from the License may be granted by purchasing a commercial license. +import copy import os import shutil from functools import reduce @@ -43,12 +44,12 @@ def __init__(self) -> None: ) + "/covalent" self.config_file = f"{config_dir}/covalent.conf" + self.generate_default_config() if os.path.exists(self.config_file): - self.read_config() + self.update_config() else: Path(config_dir).mkdir(parents=True, exist_ok=True) - self.generate_default_config() self.write_config() Path(self.get("sdk.log_dir")).mkdir(parents=True, exist_ok=True) @@ -70,7 +71,30 @@ def generate_default_config(self) -> None: from .defaults import _DEFAULT_CONFIG - self.config_data = _DEFAULT_CONFIG + # self.config_data may be modified later, and we don't want it to affect _DEFAULT_CONFIG + self.config_data = copy.deepcopy(_DEFAULT_CONFIG) + + def update_config(self) -> None: + """ + Update the exising configuration dictionary with the configuration stored in file. + + Args: + None + + Returns: + None + """ + + def update_nested_dict(old_dict, new_dict): + for key, value in new_dict.items(): + if isinstance(value, dict) and key in old_dict and isinstance(old_dict[key], dict): + update_nested_dict(old_dict[key], value) + else: + old_dict[key] = value + + if os.path.exists(self.config_file): + file_config = toml.load(self.config_file) + update_nested_dict(self.config_data, file_config) def read_config(self) -> None: """ diff --git a/doc/source/how_to/execution/choosing_conda_environments.ipynb b/doc/source/how_to/execution/choosing_conda_environments.ipynb index c294a03c7..9a64473a1 100644 --- a/doc/source/how_to/execution/choosing_conda_environments.ipynb +++ b/doc/source/how_to/execution/choosing_conda_environments.ipynb @@ -162,9 +162,9 @@ ], "metadata": { "kernelspec": { - "display_name": "Python 3 (ipykernel)", + "display_name": "cova", "language": "python", - "name": "python3" + "name": "cova" }, "language_info": { "codemirror_mode": { @@ -176,7 +176,7 @@ "name": "python", "nbconvert_exporter": "python", "pygments_lexer": "ipython3", - "version": "3.8.8" + "version": "3.8.12" } }, "nbformat": 4, diff --git a/tests/covalent_tests/shared_files/config_test.py b/tests/covalent_tests/shared_files/config_test.py new file mode 100644 index 000000000..b717debf1 --- /dev/null +++ b/tests/covalent_tests/shared_files/config_test.py @@ -0,0 +1,127 @@ +# Copyright 2021 Agnostiq Inc. +# +# This file is part of Covalent. +# +# Licensed under the GNU Affero General Public License 3.0 (the "License"). +# A copy of the License may be obtained with this software package or at +# +# https://www.gnu.org/licenses/agpl-3.0.en.html +# +# Use of this file is prohibited except in compliance with the License. Any +# modifications or derivative works of this file must retain this copyright +# notice, and modified files must contain a notice indicating that they have +# been altered from the originals. +# +# Covalent is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the License for more details. +# +# Relief from the License may be granted by purchasing a commercial license. + +import os +from unittest.mock import patch + +from covalent._shared_files.config import _ConfigManager + +CONFIG_DIR = os.path.join(os.path.dirname(__file__), "test_files") + + +@patch.dict(os.environ, {"COVALENT_CONFIG_DIR": CONFIG_DIR}, clear=True) +def test_read_config(): + """Test that configuration file is properly read""" + + config_manager = _ConfigManager() + config_manager.read_config() + + expected_dict = { + "sdk": {"log_dir": "./", "log_level": "warning", "enable_logging": "false"}, + "executors": { + "local": { + "log_stdout": "stdout.log", + "log_stderr": "stderr.log", + "cache_dir": "/tmp/covalent", + "other_params": {"name": "Local Executor", "proprietary": "False"}, + }, + "local_executor": "local.py", + "slurm_executor": "slurmp.py", + "ssh_executor": "ssh_executor.py", + }, + "test_dict": { + "1": 2, + "3": 4, + }, + } + + assert config_manager.config_data == expected_dict + + +@patch.dict(os.environ, {"COVALENT_CONFIG_DIR": CONFIG_DIR}, clear=True) +def test_update_config(): + """Test that updating the existing config data with the config file works""" + + config_manager = _ConfigManager() + config_manager.config_data = { + "sdk": { + "test_dir": "/tmp", + "log_dir": { + "dir_one": "/var", + "dir_two": "~/.log_dir", + }, + "log_level": "debug", + "enable_logging": "true", + }, + "executors": { + "local": { + "other_params": { + "proprietary": "true", + "params_test_dict": { + "a": "b", + "c": "d", + }, + } + }, + "executor_test_dict": { + "alpha": "beta", + "gamma": "delta", + }, + }, + "test_dict": None, + } + config_manager.update_config() + + expected_dict = { + "sdk": { + "test_dir": "/tmp", + "log_dir": "./", + "log_level": "warning", + "enable_logging": "false", + }, + "executors": { + "local": { + "log_stdout": "stdout.log", + "log_stderr": "stderr.log", + "cache_dir": "/tmp/covalent", + "other_params": { + "name": "Local Executor", + "proprietary": "False", + "params_test_dict": { + "a": "b", + "c": "d", + }, + }, + }, + "local_executor": "local.py", + "slurm_executor": "slurmp.py", + "ssh_executor": "ssh_executor.py", + "executor_test_dict": { + "alpha": "beta", + "gamma": "delta", + }, + }, + "test_dict": { + "1": 2, + "3": 4, + }, + } + + assert config_manager.config_data == expected_dict diff --git a/tests/covalent_tests/shared_files/test_files/covalent/covalent.conf b/tests/covalent_tests/shared_files/test_files/covalent/covalent.conf new file mode 100644 index 000000000..df1cf381f --- /dev/null +++ b/tests/covalent_tests/shared_files/test_files/covalent/covalent.conf @@ -0,0 +1,22 @@ +[sdk] +log_dir = "./" +log_level = "warning" +enable_logging = "false" + +[executors.local] +log_stdout = "stdout.log" +log_stderr = "stderr.log" +cache_dir = "/tmp/covalent" + +[executors] +local_executor = "local.py" +slurm_executor = "slurmp.py" +ssh_executor = "ssh_executor.py" + +[executors.local.other_params] +name = "Local Executor" +proprietary = "False" + +[test_dict] +1 = 2 +3 = 4 diff --git a/tests/functional_tests/covalent.conf b/tests/functional_tests/covalent.conf new file mode 100644 index 000000000..df1cf381f --- /dev/null +++ b/tests/functional_tests/covalent.conf @@ -0,0 +1,22 @@ +[sdk] +log_dir = "./" +log_level = "warning" +enable_logging = "false" + +[executors.local] +log_stdout = "stdout.log" +log_stderr = "stderr.log" +cache_dir = "/tmp/covalent" + +[executors] +local_executor = "local.py" +slurm_executor = "slurmp.py" +ssh_executor = "ssh_executor.py" + +[executors.local.other_params] +name = "Local Executor" +proprietary = "False" + +[test_dict] +1 = 2 +3 = 4