feat(config): Add global config validation (#2664)

Author: haakonvt

cognite/client/config.py

@@ -8,7 +8,7 @@
 
 from cognite.client._version import __api_subversion__
 from cognite.client.credentials import CredentialProvider
-from cognite.client.utils._auxiliary import load_resource_to_dict
+from cognite.client.utils._auxiliary import is_finite, is_positive, load_resource_to_dict
 from cognite.client.utils._concurrency import ConcurrencySettings
 from cognite.client.utils._importing import local_import
 
@@ -78,6 +78,22 @@ def __init__(self) -> None:
         self.file_upload_chunk_size: int | None = None
         self.silence_feature_preview_warnings: bool = False
 
+    def __setattr__(self, name: str, value: Any) -> None:
+        # Why __setattr__ instead of just more use of @property? It is to avoid breaking a bunch of existing
+        # inspection code (which would then need special handling). Setting global config options is a rare
+        # one-off type event, so overhead is no issue.
+        match name:
+            case "max_retries" | "max_retries_connect" | "max_retry_backoff" if not is_finite(value):
+                raise ValueError(f"{name} must be a non-negative integer, got {value!r}")
+
+            case "max_connection_pool_size" if not is_positive(value):
+                raise ValueError(f"max_connection_pool_size must be a positive integer, got {value!r}")
+
+            case "file_download_chunk_size" | "file_upload_chunk_size" if value is not None and not is_positive(value):
+                raise ValueError(f"{name} must be a positive integer or None, got {value!r}")
+
+        super().__setattr__(name, value)
+
     @property
     def max_workers(self) -> int:
         return self._max_workers
@@ -144,7 +160,15 @@ def apply_settings(self, settings: dict[str, Any] | str) -> None:
             if not isinstance(loaded["default_client_config"], ClientConfig):
                 loaded["default_client_config"] = ClientConfig.load(loaded["default_client_config"])
 
-        current_settings.update(loaded)
+        # Snapshot before applying so a mid-loop validation error can be rolled back atomically:
+        snapshot = {key: getattr(self, key) for key in loaded}
+        try:
+            for key, val in loaded.items():
+                setattr(self, key, val)
+        except (ValueError, TypeError):
+            for key, val in snapshot.items():
+                setattr(self, key, val)
+            raise
         # Deprecated, stored using a property, hence the special treatment:
         if maybe_max_workers is not None:
             self.max_workers = maybe_max_workers

tests/tests_unit/test_config.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+from collections import OrderedDict
 from contextlib import nullcontext as does_not_raise
 
 import pytest
@@ -57,6 +58,60 @@ def test_apply_settings(self, monkeypatch: MonkeyPatch, client_config: ClientCon
             assert isinstance(gc.default_client_config.credentials, Token)
             assert gc.default_client_config.project == "test-project"
 
+    @pytest.mark.parametrize(
+        "attr, value",
+        [
+            ("max_retries", 0),
+            ("max_retries", 10),
+            ("max_retries_connect", 0),
+            ("max_retries_connect", 3),
+            ("max_retry_backoff", 0),
+            ("max_retry_backoff", 60),
+            ("max_connection_pool_size", 1),
+            ("max_connection_pool_size", 20),
+            ("file_download_chunk_size", None),
+            ("file_download_chunk_size", 1024),
+            ("file_upload_chunk_size", None),
+            ("file_upload_chunk_size", 65536),
+        ],
+    )
+    def test_validated_attrs_valid(self, monkeypatch: MonkeyPatch, attr: str, value: object) -> None:
+        # raising=True ensures the attribute actually exists on global_config, catching typos in parameters
+        monkeypatch.setattr(global_config, attr, value, raising=True)
+        assert getattr(global_config, attr) == value
+
+    @pytest.mark.parametrize(
+        "attr, value, match",
+        [
+            ("max_retries", -1, "non-negative integer"),
+            ("max_retries", 1.5, "non-negative integer"),
+            ("max_retries", None, "non-negative integer"),
+            ("max_retries_connect", -1, "non-negative integer"),
+            ("max_retry_backoff", -1, "non-negative integer"),
+            ("max_connection_pool_size", 0, "positive integer"),
+            ("max_connection_pool_size", -1, "positive integer"),
+            ("file_download_chunk_size", 0, "positive integer or None"),
+            ("file_download_chunk_size", -1, "positive integer or None"),
+            ("file_upload_chunk_size", 0, "positive integer or None"),
+        ],
+    )
+    def test_validated_attrs_invalid(self, attr: str, value: object, match: str) -> None:
+        with pytest.raises(ValueError, match=match):
+            setattr(global_config, attr, value)
+
+    def test_apply_settings_invalid_value_is_rolled_back(self) -> None:
+        original_retries = global_config.max_retries
+        original_retries_connect = global_config.max_retries_connect
+        # OrderedDict guarantees max_retries (valid) is applied before max_retries_connect (invalid),
+        # so the rollback has something to actually undo:
+        with pytest.raises(ValueError, match="non-negative integer"):
+            global_config.apply_settings(
+                OrderedDict([("max_retries", original_retries + 1), ("max_retries_connect", -1)])
+            )
+
+        assert global_config.max_retries == original_retries
+        assert global_config.max_retries_connect == original_retries_connect
+
     def test_load_non_existent_attr(self) -> None:
         settings = {
             "max_workers": 0,  # use a nonsensical value to ensure that it is not applied without assuming other tests kept the default value