From 935ac11724adf5f03760bfec87f3070262e2e466 Mon Sep 17 00:00:00 2001 From: Carsten Ehbrecht Date: Thu, 18 Jun 2026 22:09:00 +0200 Subject: [PATCH] cleaned up config usage --- src/rook/__init__.py | 2 -- src/rook/catalog/__init__.py | 4 ++-- src/rook/catalog/intake.py | 4 ++-- src/rook/config.py | 18 +++++------------- src/rook/director/director.py | 6 +++--- tests/conftest.py | 7 ------- tests/test_catalog_base.py | 8 ++++---- tests/test_config.py | 18 ++++++++++-------- tests/test_ops_consolidate.py | 2 +- tests/test_ops_helpers.py | 12 ++++++------ 10 files changed, 33 insertions(+), 48 deletions(-) diff --git a/src/rook/__init__.py b/src/rook/__init__.py index 4ebcd97b..adfb0568 100644 --- a/src/rook/__init__.py +++ b/src/rook/__init__.py @@ -20,6 +20,4 @@ from .__version__ import __author__, __email__, __version__ # noqa: F401 -from .config import CONFIG # noqa: F401 - from .wsgi import application # noqa diff --git a/src/rook/catalog/__init__.py b/src/rook/catalog/__init__.py index efd7041a..38b954f5 100644 --- a/src/rook/catalog/__init__.py +++ b/src/rook/catalog/__init__.py @@ -1,12 +1,12 @@ from clisops.exceptions import InvalidCollection -from rook import CONFIG +from rook import config from .db import DBCatalog def get_catalog(project): - if CONFIG[f"project:{project}"].get("use_catalog"): + if config.get_project_config(project).get("use_catalog"): try: catalog = DBCatalog(project) return catalog diff --git a/src/rook/catalog/intake.py b/src/rook/catalog/intake.py index 162e8385..dbf84dce 100644 --- a/src/rook/catalog/intake.py +++ b/src/rook/catalog/intake.py @@ -5,7 +5,7 @@ import fsspec import intake -from rook import CONFIG +from rook import config from .base import Catalog from .util import MAX_DATETIME, MIN_DATETIME, parse_time @@ -23,7 +23,7 @@ def __init__(self, project, url=None): super().__init__(project) self.url = ( url - or CONFIG.get("catalog", {}).get("intake_catalog_url") + or config.get_config().get("catalog", {}).get("intake_catalog_url") or DEFAULT_INTAKE_CATALOG_URL ) self._cat = None diff --git a/src/rook/config.py b/src/rook/config.py index d522bca6..9271323a 100644 --- a/src/rook/config.py +++ b/src/rook/config.py @@ -1,7 +1,6 @@ """Central access to Rook configuration.""" import json -import sys from pathlib import Path from typing import Any @@ -10,27 +9,20 @@ _PACKAGE_FILE = Path(__file__) -CONFIG = _get_clisops_config(_PACKAGE_FILE) +_CONFIG = _get_clisops_config(_PACKAGE_FILE) def get_config() -> dict[str, Any]: """Return the current Rook configuration.""" - return CONFIG + return _CONFIG def reload_config() -> dict[str, Any]: """Reload Rook configuration from the standard clisops sources.""" - global CONFIG + global _CONFIG - CONFIG = _reload_clisops_config(_PACKAGE_FILE) - - # Keep the public compatibility alias current without making config depend - # on importing the rest of Rook. - rook_module = sys.modules.get("rook") - if rook_module is not None: - rook_module.CONFIG = CONFIG - - return CONFIG + _CONFIG = _reload_clisops_config(_PACKAGE_FILE) + return _CONFIG def get_project_config(project: str) -> dict[str, Any]: diff --git a/src/rook/director/director.py b/src/rook/director/director.py index 796c3aa4..a6341ef4 100644 --- a/src/rook/director/director.py +++ b/src/rook/director/director.py @@ -4,7 +4,7 @@ from clisops.project_utils import get_project_name from clisops.utils.file_utils import FileMapper -from rook import CONFIG +from rook import config from clisops.exceptions import InvalidCollection from rook.catalog import get_catalog @@ -35,7 +35,7 @@ def __init__(self, coll, inputs): self.output_uris = None self.search_result = None - if CONFIG[f"project:{self.project}"].get("use_catalog"): + if config.get_project_config(self.project).get("use_catalog"): try: self.catalog = get_catalog(self.project) except Exception: @@ -48,7 +48,7 @@ def __init__(self, coll, inputs): def use_fixes(self): # TODO: don't use fixes return False - # return CONFIG[f"project:{self.project}"].get("use_fixes", False) + # return config.get_project_config(self.project).get("use_fixes", False) def _check_apply_fixes(self): if ( diff --git a/tests/conftest.py b/tests/conftest.py index 766f9793..e0619cd3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -77,19 +77,12 @@ def write_roocs_cfg(stratus): # TODO: reload configs in clisops # workaround ... fix code in new clisops. import clisops - import rook.catalog - import rook.catalog.intake - import rook.director.director from rook.config import reload_config cfg = reload_config() clisops.CONFIG = cfg clisops.project_utils.CONFIG = cfg - rook.director.director.CONFIG = cfg - rook.catalog.CONFIG = cfg - rook.catalog.intake.CONFIG = cfg # print("clisops.config", clisops.CONFIG["project:cmip5"]["base_dir"]) - # print("rook.config", rook.CONFIG["project:cmip6"]["base_dir"]) @pytest.fixture diff --git a/tests/test_catalog_base.py b/tests/test_catalog_base.py index c37a50a1..6718832e 100644 --- a/tests/test_catalog_base.py +++ b/tests/test_catalog_base.py @@ -12,7 +12,7 @@ def test_result_files_uses_project_base_dir(monkeypatch): monkeypatch.setattr( config, - "CONFIG", + "_CONFIG", {"project:c3s-cmip6": {"base_dir": "/data/CMIP6"}}, ) @@ -26,7 +26,7 @@ def test_result_files_uses_project_base_dir(monkeypatch): def test_result_files_uses_global_s3_base_dir(monkeypatch): monkeypatch.setattr( config, - "CONFIG", + "_CONFIG", { "project:c3s-cmip6": {"base_dir": "/data/CMIP6"}, "s3": {"base_dir": "s3://example-bucket/data/CMIP6"}, @@ -45,7 +45,7 @@ def test_result_files_uses_global_s3_base_dir(monkeypatch): def test_result_files_uses_project_s3_base_dir_override(monkeypatch): monkeypatch.setattr( config, - "CONFIG", + "_CONFIG", { "project:c3s-cmip6": { "base_dir": "/data/CMIP6", @@ -67,7 +67,7 @@ def test_result_files_uses_project_s3_base_dir_override(monkeypatch): def test_result_download_urls_keep_data_node_root(monkeypatch): monkeypatch.setattr( config, - "CONFIG", + "_CONFIG", { "project:c3s-cmip6": { "base_dir": "/data/CMIP6", diff --git a/tests/test_config.py b/tests/test_config.py index 08062d69..83ec17aa 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -2,10 +2,14 @@ from rook import config +def test_rook_has_no_config_compatibility_alias(): + assert not hasattr(rook, "CONFIG") + + def test_get_project_config(monkeypatch): monkeypatch.setattr( config, - "CONFIG", + "_CONFIG", {"project:demo": {"base_dir": "/data/demo"}}, ) @@ -16,7 +20,7 @@ def test_get_project_config(monkeypatch): def test_get_storage_base_prefers_project_s3_override(monkeypatch): monkeypatch.setattr( config, - "CONFIG", + "_CONFIG", { "project:demo": { "base_dir": "/data/demo", @@ -32,7 +36,7 @@ def test_get_storage_base_prefers_project_s3_override(monkeypatch): def test_get_storage_base_falls_back_to_local_project_root(monkeypatch): monkeypatch.setattr( config, - "CONFIG", + "_CONFIG", {"project:demo": {"base_dir": "/data/demo"}}, ) @@ -42,19 +46,17 @@ def test_get_storage_base_falls_back_to_local_project_root(monkeypatch): def test_s3_options_ignore_malformed_optional_json(monkeypatch): monkeypatch.setattr( config, - "CONFIG", + "_CONFIG", {"s3": {"storage_options_json": "not-json", "anon": "false"}}, ) assert config.get_s3_storage_options() == {"anon": False} -def test_reload_config_updates_compatibility_alias(monkeypatch): +def test_reload_config_updates_current_config(monkeypatch): reloaded = {"project:demo": {"base_dir": "/new/data"}} - monkeypatch.setattr(config, "CONFIG", config.CONFIG) - monkeypatch.setattr(rook, "CONFIG", rook.CONFIG) + monkeypatch.setattr(config, "_CONFIG", config.get_config()) monkeypatch.setattr(config, "_reload_clisops_config", lambda _path: reloaded) assert config.reload_config() is reloaded assert config.get_config() is reloaded - assert rook.CONFIG is reloaded diff --git a/tests/test_ops_consolidate.py b/tests/test_ops_consolidate.py index e567a610..840c4b84 100644 --- a/tests/test_ops_consolidate.py +++ b/tests/test_ops_consolidate.py @@ -81,7 +81,7 @@ def search(self, collection, time): monkeypatch.setattr(consolidate, "get_catalog", lambda _project: DummyCatalog()) monkeypatch.setattr( config, - "CONFIG", + "_CONFIG", { "project:c3s-cmip6": {"base_dir": "/data/CMIP6"}, "s3": {"base_dir": "s3://example-bucket/data/CMIP6"}, diff --git a/tests/test_ops_helpers.py b/tests/test_ops_helpers.py index d1219d8b..c269b5cd 100644 --- a/tests/test_ops_helpers.py +++ b/tests/test_ops_helpers.py @@ -228,7 +228,7 @@ def fake_open_zarr(store, **kwargs): monkeypatch.setattr(helpers.xr, "open_zarr", fake_open_zarr) monkeypatch.setattr( config, - "CONFIG", + "_CONFIG", {"s3": {"anon": "true", "endpoint_url": "https://s3.example.org"}}, ) @@ -267,7 +267,7 @@ def fail_apply_fixes(_ds_id, _ds): def test_get_storage_options_for_s3_source(monkeypatch): monkeypatch.setattr( config, - "CONFIG", + "_CONFIG", {"s3": {"anon": "true", "endpoint_url": "https://s3.example.org"}}, ) @@ -280,7 +280,7 @@ def test_get_storage_options_for_s3_source(monkeypatch): def test_get_storage_options_without_s3_config(monkeypatch): - monkeypatch.setattr(config, "CONFIG", {}) + monkeypatch.setattr(config, "_CONFIG", {}) options = helpers.get_storage_options(source(None, "s3://bucket/file.nc")) @@ -290,7 +290,7 @@ def test_get_storage_options_without_s3_config(monkeypatch): def test_get_storage_options_does_not_depend_on_format(monkeypatch): monkeypatch.setattr( config, - "CONFIG", + "_CONFIG", {"s3": {"anon": "true", "endpoint_url": "https://s3.example.org"}}, ) @@ -308,7 +308,7 @@ def fake_open(path, **kwargs): return "DATASET" monkeypatch.setattr(helpers, "open_xr_dataset", fake_open) - monkeypatch.setattr(config, "CONFIG", {"s3": {"anon": "true"}}) + monkeypatch.setattr(config, "_CONFIG", {"s3": {"anon": "true"}}) result = helpers.open_dataset( source(None, "s3://bucket/reference.json"), apply_fixes=False @@ -339,7 +339,7 @@ def fake_open(file_paths, **kwargs): monkeypatch.setattr(helpers, "apply_dataset_fixes", lambda _ds_id, ds: ds) monkeypatch.setattr( config, - "CONFIG", + "_CONFIG", {"s3": {"anon": "true", "endpoint_url": "https://s3.example.org"}}, )