Skip to content

Commit

Permalink
Fix missing config keys (#92)
Browse files Browse the repository at this point in the history
* 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/.
  • Loading branch information
jkanem authored Feb 7, 2022
1 parent 5d3e45e commit fb7f432
Show file tree
Hide file tree
Showing 7 changed files with 208 additions and 7 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0.24.7
0.24.8
30 changes: 27 additions & 3 deletions covalent/_shared_files/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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:
"""
Expand Down
6 changes: 3 additions & 3 deletions doc/source/how_to/execution/choosing_conda_environments.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,9 @@
],
"metadata": {
"kernelspec": {
"display_name": "Python 3 (ipykernel)",
"display_name": "cova",
"language": "python",
"name": "python3"
"name": "cova"
},
"language_info": {
"codemirror_mode": {
Expand All @@ -176,7 +176,7 @@
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.8.8"
"version": "3.8.12"
}
},
"nbformat": 4,
Expand Down
127 changes: 127 additions & 0 deletions tests/covalent_tests/shared_files/config_test.py
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
22 changes: 22 additions & 0 deletions tests/functional_tests/covalent.conf
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit fb7f432

Please sign in to comment.