From 2c5dbd572f5250a371f0311650e969b65984ce83 Mon Sep 17 00:00:00 2001 From: sol1105 Date: Mon, 19 May 2025 19:40:44 +0200 Subject: [PATCH 1/3] config.py: Added reload function and debug statements to load_config --- .../data_request_api/utilities/config.py | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/data_request_api/data_request_api/utilities/config.py b/data_request_api/data_request_api/utilities/config.py index 8d8186f8..995f4839 100644 --- a/data_request_api/data_request_api/utilities/config.py +++ b/data_request_api/data_request_api/utilities/config.py @@ -1,10 +1,12 @@ #!/usr/bin/env python -from importlib.metadata import version, PackageNotFoundError import os +from importlib.metadata import PackageNotFoundError, version from pathlib import Path + import requests import yaml +from data_request_api.utilities.logger import get_logger # noqa # Config file location in the user's home directory PACKAGE_NAME = "CMIP7_data_request_api" @@ -71,8 +73,11 @@ def _sanity_check(key, value): ) -def load_config() -> dict: +def load_config(reload=False) -> dict: """Load the configuration file, creating it if necessary. + Args: + reload (bool): Reload config from disk and do not load from cache. + Default is False. Returns: dict: The configuration data. @@ -85,7 +90,12 @@ def load_config() -> dict: ValueError: If the value is not within the valid values for the key. """ global CONFIG + logger = get_logger() + + if reload: + CONFIG = {} if CONFIG == {}: + logger.debug(f"Loading config file: {CONFIG_FILE}") try: with open(CONFIG_FILE) as f: CONFIG = yaml.safe_load(f) @@ -99,20 +109,19 @@ def load_config() -> dict: yaml.dump(DEFAULT_CONFIG, f) CONFIG = DEFAULT_CONFIG.copy() elif not isinstance(CONFIG, dict): - raise TypeError( - f"Config file ('{CONFIG_FILE}') must contain a dictionary" - ) + raise TypeError(f"Config file ('{CONFIG_FILE}') must contain a dictionary") # Sanity test for allowed types and values for key, value in CONFIG.items(): _sanity_check(key, value) # Ensure all required keys are present and update config file if necessary - missing_keys = { - k: v for k, v in DEFAULT_CONFIG.items() if k not in CONFIG - } + missing_keys = {k: v for k, v in DEFAULT_CONFIG.items() if k not in CONFIG} for key, value in missing_keys.items(): update_config(key, value) + logger.debug(f"Loaded config from file: {CONFIG}") + else: + logger.debug(f"Loaded config from cache: {CONFIG}") return CONFIG @@ -169,7 +178,7 @@ def check_api_version(): try: response = requests.get(f"https://pypi.org/pypi/{PACKAGE_NAME}/json", timeout=5) response.raise_for_status() - latest_version = response.json()['info']['version'] + latest_version = response.json()["info"]["version"] except requests.RequestException as e: print(f"Error checking PyPI: {e}") return @@ -183,7 +192,7 @@ def check_api_version(): msg += f" pip install --upgrade {PACKAGE_NAME}\n" msg += "To turn off this warning:\n" msg += " CMIP7_data_request_api_config check_api_version false" - msg = '\n' + msg + '\n' + msg = "\n" + msg + "\n" # Add color to the warning message color_code = "\033[91m" From a74d2775e38b387cbc8080e7a8b83f5bd6c48fd5 Mon Sep 17 00:00:00 2001 From: sol1105 Date: Mon, 19 May 2025 19:42:33 +0200 Subject: [PATCH 2/3] dreq_content.py: Added debug statement to retrieve --- data_request_api/data_request_api/content/dreq_content.py | 1 + 1 file changed, 1 insertion(+) diff --git a/data_request_api/data_request_api/content/dreq_content.py b/data_request_api/data_request_api/content/dreq_content.py index 0579aa3c..d956d5c3 100644 --- a/data_request_api/data_request_api/content/dreq_content.py +++ b/data_request_api/data_request_api/content/dreq_content.py @@ -476,6 +476,7 @@ def retrieve(version="latest_stable", **kwargs): # Store the path to the dreq.json in the json_paths dictionary json_paths[version] = json_path + logger.debug(f"'{version}' stored under '{json_path}'") # Capture no correct export found for cached versions (offline mode) if not json_paths or json_paths == {}: From 4aa592a5a436ad4c3bdb8063b9325dd77a4980a2 Mon Sep 17 00:00:00 2001 From: sol1105 Date: Mon, 19 May 2025 19:44:23 +0200 Subject: [PATCH 3/3] test_cli.py: Revert os.chdir after test, set neutral state at beginning of each test --- .../data_request_api/tests/test_cli.py | 418 ++++++++++-------- 1 file changed, 231 insertions(+), 187 deletions(-) diff --git a/data_request_api/data_request_api/tests/test_cli.py b/data_request_api/data_request_api/tests/test_cli.py index 51605342..a5d3107a 100644 --- a/data_request_api/data_request_api/tests/test_cli.py +++ b/data_request_api/data_request_api/tests/test_cli.py @@ -5,6 +5,7 @@ from pathlib import Path import data_request_api.content.dreq_content as dc +import data_request_api.utilities.config as dreqcfg import pytest import yaml @@ -51,6 +52,10 @@ def setup_method(self, request): "cache_dir": str(self.temp_config_file.parent), } yaml.dump(config, fh) + dc._dreq_res = self.temp_config_file.parent + dc.versions = {"tags": [], "branches": []} + dreqcfg.CONFIG_FILE = self.temp_config_file + dreqcfg.CONFIG = {} # alternatively: dreqcfg.load_config(reload=True) dc.load("v1.2") def test_export_dreq_lists_json(self, temp_config_file, consolidate): @@ -71,7 +76,9 @@ def test_export_dreq_lists_json(self, temp_config_file, consolidate): assert result.returncode == 0 assert os.path.exists(ofile) and os.path.getsize(ofile) > 0 - def test_export_dreq_lists_json_with_opportunities_file(self, temp_config_file, consolidate): + def test_export_dreq_lists_json_with_opportunities_file( + self, temp_config_file, consolidate + ): # Test that the script creates an opportunities file template opportunities_file = temp_config_file.parent / "opportunities.json" opportunities_file.unlink(missing_ok=True) @@ -91,7 +98,10 @@ def test_export_dreq_lists_json_with_opportunities_file(self, temp_config_file, text=True, ) assert result.returncode == 0 - assert os.path.exists(opportunities_file) and os.path.getsize(opportunities_file) > 0 + assert ( + os.path.exists(opportunities_file) + and os.path.getsize(opportunities_file) > 0 + ) assert not os.path.exists(ofile) or os.path.getsize(ofile) == 0 # Test that it now applies the opportunities settings from opportunities_file @@ -111,7 +121,9 @@ def test_export_dreq_lists_json_with_opportunities_file(self, temp_config_file, assert result.returncode == 0 assert os.path.exists(ofile) and os.path.getsize(ofile) > 0 - def test_export_dreq_lists_json_with_invalid_opportunities_file(self, temp_config_file, consolidate): + def test_export_dreq_lists_json_with_invalid_opportunities_file( + self, temp_config_file, consolidate + ): # Test that the script raises an error with an invalid opportunities file opportunities_file = temp_config_file.parent / "invalid_opportunities.json" opportunities_file.unlink(missing_ok=True) @@ -170,6 +182,10 @@ def setup_method(self, request): "cache_dir": str(self.temp_config_file.parent), } yaml.dump(config, fh) + dc._dreq_res = self.temp_config_file.parent + dc.versions = {"tags": [], "branches": []} + dreqcfg.CONFIG_FILE = self.temp_config_file + dreqcfg.CONFIG = {} # alternatively: dreqcfg.load_config(reload=True) dc.load("v1.2") def test_get_variables_metadata(self, temp_config_file, consolidate): @@ -190,7 +206,9 @@ def test_get_variables_metadata(self, temp_config_file, consolidate): assert result.returncode == 0 assert os.path.exists(ofile) and os.path.getsize(ofile) > 0 - def test_get_variables_metadata_with_compound_names(self, temp_config_file, consolidate): + def test_get_variables_metadata_with_compound_names( + self, temp_config_file, consolidate + ): ofile = temp_config_file.parent / "test2.json" ofile.unlink(missing_ok=True) result = subprocess.run( @@ -211,7 +229,9 @@ def test_get_variables_metadata_with_compound_names(self, temp_config_file, cons assert result.returncode == 0 assert os.path.exists(ofile) and os.path.getsize(ofile) > 0 - def test_get_variables_metadata_with_cmor_tables(self, temp_config_file, consolidate): + def test_get_variables_metadata_with_cmor_tables( + self, temp_config_file, consolidate + ): ofile = temp_config_file.parent / "test3.json" ofile.unlink(missing_ok=True) result = subprocess.run( @@ -232,7 +252,9 @@ def test_get_variables_metadata_with_cmor_tables(self, temp_config_file, consoli assert result.returncode == 0 assert os.path.exists(ofile) and os.path.getsize(ofile) > 0 - def test_get_variables_metadata_with_cmor_variables(self, temp_config_file, consolidate): + def test_get_variables_metadata_with_cmor_variables( + self, temp_config_file, consolidate + ): ofile = temp_config_file.parent / "test4.json" ofile.unlink(missing_ok=True) result = subprocess.run( @@ -288,124 +310,138 @@ def setup_method(self, request): "cache_dir": str(self.temp_config_file.parent), } yaml.dump(config, fh) + dc._dreq_res = self.temp_config_file.parent + dc.versions = {"tags": [], "branches": []} + dreqcfg.CONFIG_FILE = self.temp_config_file + dreqcfg.CONFIG = {} # alternatively: dreqcfg.load_config(reload=True) dc.load("v1.2") dc.load("v1.2.1") def test_compare_variables(self, temp_config_file, consolidate): - os.chdir(temp_config_file.parent) - ofileA = temp_config_file.parent / "testA.json" - ofileB = temp_config_file.parent / "testB.json" - ofile_vars = temp_config_file.parent / "diffs_by_variable.json" - ofile_attr = temp_config_file.parent / "diffs_by_attribute.json" - ofile_missing = temp_config_file.parent / "missing_variables.json" - attr_file = temp_config_file.parent / "attributes.yaml" + prev_cwd = os.getcwd() + try: + os.chdir(temp_config_file.parent) + ofileA = temp_config_file.parent / "testA.json" + ofileB = temp_config_file.parent / "testB.json" + ofile_vars = temp_config_file.parent / "diffs_by_variable.json" + ofile_attr = temp_config_file.parent / "diffs_by_attribute.json" + ofile_missing = temp_config_file.parent / "missing_variables.json" + attr_file = temp_config_file.parent / "attributes.yaml" - # Part 1 - Standard comparison - ofile_vars.unlink(missing_ok=True) - ofile_attr.unlink(missing_ok=True) - ofile_missing.unlink(missing_ok=True) - ofileA.unlink(missing_ok=True) - ofileB.unlink(missing_ok=True) - attr_file.unlink(missing_ok=True) - # Create Variable List A - result = subprocess.run( - [ - sys.executable, - "-m", - "data_request_api.command_line.get_variables_metadata", - "v1.2", - "-o", - ofileA, - ], - capture_output=True, - text=True, - ) - assert result.returncode == 0 - assert os.path.exists(ofileA) and os.path.getsize(ofileA) > 0 - # Create Variable List B - result = subprocess.run( - [ - sys.executable, - "-m", - "data_request_api.command_line.get_variables_metadata", - "v1.2.1", - "-o", - ofileB, - ], - capture_output=True, - text=True, - ) - assert result.returncode == 0 - assert os.path.exists(ofileB) and os.path.getsize(ofileB) > 0 - # Actual comparison - result = subprocess.run( - [ - sys.executable, - "-m", - "data_request_api.command_line.compare_variables", - ofileA, - ofileB, - ], - capture_output=True, - text=True, - ) - assert result.returncode == 0 - assert os.path.exists(ofile_missing) and os.path.getsize(ofile_missing) > 0 - assert os.path.exists(ofile_vars) and os.path.getsize(ofile_vars) > 0 - assert os.path.exists(ofile_attr) and os.path.getsize(ofile_attr) > 0 - assert os.path.exists(attr_file) and os.path.getsize(attr_file) > 0 + # Part 1 - Standard comparison + ofile_vars.unlink(missing_ok=True) + ofile_attr.unlink(missing_ok=True) + ofile_missing.unlink(missing_ok=True) + ofileA.unlink(missing_ok=True) + ofileB.unlink(missing_ok=True) + attr_file.unlink(missing_ok=True) + # Create Variable List A + result = subprocess.run( + [ + sys.executable, + "-m", + "data_request_api.command_line.get_variables_metadata", + "v1.2", + "-o", + ofileA, + ], + capture_output=True, + text=True, + ) + assert result.returncode == 0 + assert os.path.exists(ofileA) and os.path.getsize(ofileA) > 0 + # Create Variable List B + result = subprocess.run( + [ + sys.executable, + "-m", + "data_request_api.command_line.get_variables_metadata", + "v1.2.1", + "-o", + ofileB, + ], + capture_output=True, + text=True, + ) + assert result.returncode == 0 + assert os.path.exists(ofileB) and os.path.getsize(ofileB) > 0 + # Actual comparison + result = subprocess.run( + [ + sys.executable, + "-m", + "data_request_api.command_line.compare_variables", + ofileA, + ofileB, + ], + capture_output=True, + text=True, + ) + assert result.returncode == 0 + assert os.path.exists(ofile_missing) and os.path.getsize(ofile_missing) > 0 + assert os.path.exists(ofile_vars) and os.path.getsize(ofile_vars) > 0 + assert os.path.exists(ofile_attr) and os.path.getsize(ofile_attr) > 0 + assert os.path.exists(attr_file) and os.path.getsize(attr_file) > 0 - # Part 2 - Provide attribute file - ofile_vars.unlink(missing_ok=True) - ofile_attr.unlink(missing_ok=True) - ofile_missing.unlink(missing_ok=True) - # Write custom attributes file - cattr_file = temp_config_file.parent / "custom_attrs.yaml" - cattr_file.unlink(missing_ok=True) - config = { - "compare_attributes": ["standard_name", "units", "cell_methods"], - "repos": { - "cmip6": { - "url": "https://github.com/PCMDI/cmip6-cmor-tables", - } - }, - } - with open(cattr_file, "w") as f: - yaml.dump(config, f, default_flow_style=False) - # Actual comparison - result = subprocess.run( - [ - sys.executable, - "-m", - "data_request_api.command_line.compare_variables", - ofileA, - ofileB, - "-c", - cattr_file, - ], - capture_output=True, - text=True, - ) - assert result.returncode == 0 - assert os.path.exists(ofile_missing) and os.path.getsize(ofile_missing) > 0 - assert os.path.exists(ofile_vars) and os.path.getsize(ofile_vars) > 0 - assert os.path.exists(ofile_attr) and os.path.getsize(ofile_attr) > 0 - assert os.path.getsize(cattr_file) < os.path.getsize(attr_file) + # Part 2 - Provide attribute file + ofile_vars.unlink(missing_ok=True) + ofile_attr.unlink(missing_ok=True) + ofile_missing.unlink(missing_ok=True) + # Write custom attributes file + cattr_file = temp_config_file.parent / "custom_attrs.yaml" + cattr_file.unlink(missing_ok=True) + config = { + "compare_attributes": ["standard_name", "units", "cell_methods"], + "repos": { + "cmip6": { + "url": "https://github.com/PCMDI/cmip6-cmor-tables", + } + }, + } + with open(cattr_file, "w") as f: + yaml.dump(config, f, default_flow_style=False) + # Actual comparison + result = subprocess.run( + [ + sys.executable, + "-m", + "data_request_api.command_line.compare_variables", + ofileA, + ofileB, + "-c", + cattr_file, + ], + capture_output=True, + text=True, + ) + assert result.returncode == 0 + assert os.path.exists(ofile_missing) and os.path.getsize(ofile_missing) > 0 + assert os.path.exists(ofile_vars) and os.path.getsize(ofile_vars) > 0 + assert os.path.exists(ofile_attr) and os.path.getsize(ofile_attr) > 0 + assert os.path.getsize(cattr_file) < os.path.getsize(attr_file) - # Part 3 - Compare with CMIP6 - ofile_vars.unlink(missing_ok=True) - ofile_attr.unlink(missing_ok=True) - ofile_missing.unlink(missing_ok=True) - # Actual comparison - result = subprocess.run( - [sys.executable, "-m", "data_request_api.command_line.compare_variables", ofileB, "cmip6"], - capture_output=True, - text=True, - ) - assert result.returncode == 0 - assert os.path.exists(ofile_missing) and os.path.getsize(ofile_missing) > 0 - assert os.path.exists(ofile_vars) and os.path.getsize(ofile_vars) > 0 - assert os.path.exists(ofile_attr) and os.path.getsize(ofile_attr) > 0 + # Part 3 - Compare with CMIP6 + ofile_vars.unlink(missing_ok=True) + ofile_attr.unlink(missing_ok=True) + ofile_missing.unlink(missing_ok=True) + # Actual comparison + result = subprocess.run( + [ + sys.executable, + "-m", + "data_request_api.command_line.compare_variables", + ofileB, + "cmip6", + ], + capture_output=True, + text=True, + ) + assert result.returncode == 0 + assert os.path.exists(ofile_missing) and os.path.getsize(ofile_missing) > 0 + assert os.path.exists(ofile_vars) and os.path.getsize(ofile_vars) > 0 + assert os.path.exists(ofile_attr) and os.path.getsize(ofile_attr) > 0 + finally: + os.chdir(prev_cwd) @pytest.mark.parametrize( @@ -426,76 +462,84 @@ def setup_method(self, request): "cache_dir": str(self.temp_config_file.parent), } yaml.dump(config, fh) + dc._dreq_res = self.temp_config_file.parent + dc.versions = {"tags": [], "branches": []} + dreqcfg.CONFIG_FILE = self.temp_config_file + dreqcfg.CONFIG = {} # alternatively: dreqcfg.load_config(reload=True) dc.load("v1.2") def test_estimate_dreq_volume(self, temp_config_file, consolidate): - os.chdir(temp_config_file.parent) - ofile = temp_config_file.parent / "test1.json" - sizecfg = temp_config_file.parent / "size.yaml" - ofile.unlink(missing_ok=True) - sizecfg.unlink(missing_ok=True) - # Part 1 - Create size.yaml - result = subprocess.run( - [ - sys.executable, - "-m", - "data_request_api.command_line.estimate_dreq_volume", - "v1.2", - "-o", - ofile, - ], - capture_output=True, - text=True, - ) - assert result.returncode == 0 - assert not os.path.exists(ofile) or os.path.getsize(ofile) == 0 - assert os.path.exists(sizecfg) and os.path.getsize(sizecfg) > 0 - # Part 2 - Actual volume estimate - result = subprocess.run( - [ - sys.executable, - "-m", - "data_request_api.command_line.estimate_dreq_volume", - "v1.2", - "-o", - ofile, - ], - capture_output=True, - text=True, - ) - assert result.returncode == 0 - assert os.path.exists(ofile) and os.path.getsize(ofile) > 0 - assert os.path.exists(sizecfg) and os.path.getsize(sizecfg) > 0 - # Part 3 - Custom size.yaml - ofile.unlink(missing_ok=True) - csizecfg = temp_config_file.parent / "custom_size.yaml" - csizecfg.unlink(missing_ok=True) - # Read default size.yaml - with open(sizecfg) as fh: - config = yaml.safe_load(fh) - sizecfg.unlink(missing_ok=True) - # Update config - config["longitude"] = 720 - config["latitude"] = 360 - # Write custom size.yaml - with open(csizecfg, "w") as fh: - yaml.dump(config, fh) - # Actual volume estimate - result = subprocess.run( - [ - sys.executable, - "-m", - "data_request_api.command_line.estimate_dreq_volume", - "v1.2", - "-o", - ofile, - "-c", - csizecfg, - ], - capture_output=True, - text=True, - ) - assert result.returncode == 0 - assert os.path.exists(ofile) and os.path.getsize(ofile) > 0 - assert os.path.exists(csizecfg) and os.path.getsize(csizecfg) > 0 - assert not os.path.exists(sizecfg) or os.path.getsize(sizecfg) == 0 + prev_cwd = os.getcwd() + try: + os.chdir(temp_config_file.parent) + ofile = temp_config_file.parent / "test1.json" + sizecfg = temp_config_file.parent / "size.yaml" + ofile.unlink(missing_ok=True) + sizecfg.unlink(missing_ok=True) + # Part 1 - Create size.yaml + result = subprocess.run( + [ + sys.executable, + "-m", + "data_request_api.command_line.estimate_dreq_volume", + "v1.2", + "-o", + ofile, + ], + capture_output=True, + text=True, + ) + assert result.returncode == 0 + assert not os.path.exists(ofile) or os.path.getsize(ofile) == 0 + assert os.path.exists(sizecfg) and os.path.getsize(sizecfg) > 0 + # Part 2 - Actual volume estimate + result = subprocess.run( + [ + sys.executable, + "-m", + "data_request_api.command_line.estimate_dreq_volume", + "v1.2", + "-o", + ofile, + ], + capture_output=True, + text=True, + ) + assert result.returncode == 0 + assert os.path.exists(ofile) and os.path.getsize(ofile) > 0 + assert os.path.exists(sizecfg) and os.path.getsize(sizecfg) > 0 + # Part 3 - Custom size.yaml + ofile.unlink(missing_ok=True) + csizecfg = temp_config_file.parent / "custom_size.yaml" + csizecfg.unlink(missing_ok=True) + # Read default size.yaml + with open(sizecfg) as fh: + config = yaml.safe_load(fh) + sizecfg.unlink(missing_ok=True) + # Update config + config["longitude"] = 720 + config["latitude"] = 360 + # Write custom size.yaml + with open(csizecfg, "w") as fh: + yaml.dump(config, fh) + # Actual volume estimate + result = subprocess.run( + [ + sys.executable, + "-m", + "data_request_api.command_line.estimate_dreq_volume", + "v1.2", + "-o", + ofile, + "-c", + csizecfg, + ], + capture_output=True, + text=True, + ) + assert result.returncode == 0 + assert os.path.exists(ofile) and os.path.getsize(ofile) > 0 + assert os.path.exists(csizecfg) and os.path.getsize(csizecfg) > 0 + assert not os.path.exists(sizecfg) or os.path.getsize(sizecfg) == 0 + finally: + os.chdir(prev_cwd)