From aaa825a9a81625118be4335a32ee7c6bbf34dea6 Mon Sep 17 00:00:00 2001 From: skshetry <18718008+skshetry@users.noreply.github.com> Date: Fri, 10 Mar 2023 21:52:05 +0545 Subject: [PATCH] config: ignore unspecified keys from `feature` section (#9153) A warning is shown to the user for the unsupported/ignored keys, and then ignored. The warning is raised only once per interpreter session. --- dvc/config_schema.py | 50 ++++++++++++++++++++++++++++++++------- dvc/fs/__init__.py | 2 +- tests/unit/test_config.py | 20 ++++++++++++++++ 3 files changed, 63 insertions(+), 9 deletions(-) diff --git a/dvc/config_schema.py b/dvc/config_schema.py index bdfeed003..5f7d15926 100644 --- a/dvc/config_schema.py +++ b/dvc/config_schema.py @@ -1,8 +1,21 @@ +import logging import os from urllib.parse import urlparse -from funcy import walk_values -from voluptuous import All, Any, Coerce, Invalid, Lower, Optional, Range, Schema +from funcy import once, walk_values +from voluptuous import ( + REMOVE_EXTRA, + All, + Any, + Coerce, + Invalid, + Lower, + Optional, + Range, + Schema, +) + +logger = logging.getLogger(__name__) Bool = All( Lower, @@ -65,6 +78,27 @@ class RelPath(str): pass +class FeatureSchema(Schema): + def __init__(self, schema, required=False): + super().__init__(schema, required=required, extra=REMOVE_EXTRA) + + @staticmethod + @once + def _log_deprecated(keys): + # only run this once per session + message = "%s config option%s unsupported" + paths = ", ".join(f"'feature.{key}'" for key in keys) + pluralize = " is" if len(keys) == 1 else "s are" + logger.warning(message, paths, pluralize) + + def __call__(self, data): + ret = super().__call__(data) + extra_keys = data.keys() - ret.keys() + if extra_keys: + self._log_deprecated(sorted(extra_keys)) + return ret + + REMOTE_COMMON = { "url": str, "checksum_jobs": All(Coerce(int), Range(1)), @@ -262,12 +296,12 @@ SCHEMA = { }, }, # section for experimental features - "feature": { - Optional("machine", default=False): Bool, - # enabled by default. It's of no use, kept for backward compatibility. - Optional("data_index_cache", default=False): Bool, - Optional("parametrization", default=True): Bool, - }, + # only specified keys are validated, others get logged and then ignored/removed + "feature": FeatureSchema( + { + Optional("machine", default=False): Bool, + }, + ), "plots": { "html_template": str, Optional("auto_open", default=False): Bool, diff --git a/dvc/fs/__init__.py b/dvc/fs/__init__.py index ae2544de1..59ad0e5a1 100644 --- a/dvc/fs/__init__.py +++ b/dvc/fs/__init__.py @@ -121,7 +121,7 @@ def _get_cloud_fs(repo_config, **kwargs): remote_conf = get_fs_config(repo_config, **kwargs) try: - remote_conf = SCHEMA["remote"][str](remote_conf) + remote_conf = SCHEMA["remote"][str](remote_conf) # type: ignore[index] except Invalid as exc: raise RepoConfigError(str(exc)) from None diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index d0badc22f..c81efa7fc 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -1,3 +1,4 @@ +import logging import os import textwrap @@ -94,3 +95,22 @@ def test_load_configob_error(tmp_dir, dvc, mocker): with pytest.raises(ConfigError): with config.edit(): pass + + +def test_feature_section_supports_arbitrary_values(caplog): + with caplog.at_level(logging.WARNING, logger="dvc.config_schema"): + data = Config.validate( + { + "feature": { + "random_key_1": "random_value_1", + "random_key_2": 42, + } + } + ) + + assert "random_key_1" not in data + assert "random_key_2" not in data + assert ( + "'feature.random_key_1', 'feature.random_key_2' " + "config options are unsupported" + ) in caplog.text