Skip to content

Commit

Permalink
Stop OmegaConfigLoader from reading config from hidden directory like…
Browse files Browse the repository at this point in the history
… `ipynb_checkpoints` (#2977)

* Check plugins implement valid hooks

Signed-off-by: Nok Chan <nok.lam.chan@quantumblack.com>

* Add release note

Signed-off-by: Nok Chan <nok.lam.chan@quantumblack.com>

* Staging work - add custom functions to check hidden folder and files. Tests still failing

Signed-off-by: Nok Chan <nok.lam.chan@quantumblack.com>

* Fix test - checkpoints should use the same environment

Signed-off-by: Nok Chan <nok.lam.chan@quantumblack.com>

* Revert "Check plugins implement valid hooks"

This reverts commit f10bede.

* Update RELEASE.md

Co-authored-by: Ankita Katiyar <110245118+ankatiyar@users.noreply.github.com>

* fix lint

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

---------

Signed-off-by: Nok Chan <nok.lam.chan@quantumblack.com>
Signed-off-by: Nok <nok.lam.chan@quantumblack.com>
Co-authored-by: Ankita Katiyar <110245118+ankatiyar@users.noreply.github.com>
  • Loading branch information
noklam and ankatiyar committed Aug 30, 2023
1 parent 59f2cfb commit a91ccdb
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 5 deletions.
1 change: 1 addition & 0 deletions RELEASE.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
* Updated `kedro pipeline create` and `kedro catalog create` to use new `/conf` file structure.
* Converted `setup.py` in default template to `pyproject.toml` and moved flake8 configuration
to dedicated file `.flake8`.
* Updated `OmegaConfigLoader` to ignore config from hidden directories like `.ipynb_checkpoints`.

## Documentation changes
* Revised the `data` section to restructure beginner and advanced pages about the Data Catalog and datasets.
Expand Down
19 changes: 14 additions & 5 deletions kedro/config/omegaconf_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,11 +250,12 @@ def load_and_merge_dir_config( # noqa: too-many-arguments
f"or is not a valid directory: {conf_path}"
)

paths = [
Path(each)
for pattern in patterns
for each in self._fs.glob(Path(f"{str(conf_path)}/{pattern}").as_posix())
]
paths = []
for pattern in patterns:
for each in self._fs.glob(Path(f"{str(conf_path)}/{pattern}").as_posix()):
if not self._is_hidden(each):
paths.append(Path(each))

deduplicated_paths = set(paths)
config_files_filtered = [
path for path in deduplicated_paths if self._is_valid_config_path(path)
Expand Down Expand Up @@ -392,3 +393,11 @@ def _resolve_environment_variables(config: dict[str, Any]) -> None:
OmegaConf.clear_resolver("oc.env")
else:
OmegaConf.resolve(config)

def _is_hidden(self, path: str):
"""Check if path contains any hidden directory or is a hidden file"""
path = Path(path).resolve().as_posix()
parts = path.split(self._fs.sep) # filesystem specific separator
HIDDEN = "."
# Check if any component (folder or file) starts with a dot (.)
return any(part.startswith(HIDDEN) for part in parts)
40 changes: 40 additions & 0 deletions tests/config/test_omegaconf_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -797,3 +797,43 @@ def test_bad_globals_underscore(self, tmp_path):
match=r"Keys starting with '_' are not supported for globals.",
):
conf["parameters"]["param2"]

@pytest.mark.parametrize(
"hidden_path", ["/User/.hidden/dummy.yml", "/User/dummy/.hidden.yml"]
)
def test_is_hidden_config(self, tmp_path, hidden_path):
conf = OmegaConfigLoader(str(tmp_path))
assert conf._is_hidden(hidden_path)

@pytest.mark.parametrize(
"hidden_path",
[
"/User/conf/base/catalog.yml",
"/User/conf/local/catalog/data_science.yml",
"/User/notebooks/../conf/base/catalog",
],
)
def test_not_hidden_config(self, tmp_path, hidden_path):
conf = OmegaConfigLoader(str(tmp_path))
assert not conf._is_hidden(hidden_path)

def test_ignore_ipynb_checkpoints(self, tmp_path, mocker):
conf = OmegaConfigLoader(str(tmp_path), default_run_env=_BASE_ENV)
base_path = tmp_path / _BASE_ENV / "parameters.yml"
checkpoints_path = (
tmp_path / _BASE_ENV / ".ipynb_checkpoints" / "parameters.yml"
)

base_config = {"param1": "dummy"}
checkpoints_config = {"param1": "dummy"}

_write_yaml(base_path, base_config)
_write_yaml(checkpoints_path, checkpoints_config)

# read successfully
conf["parameters"]

mocker.patch.object(conf, "_is_hidden", return_value=False) #
with pytest.raises(ValueError, match="Duplicate keys found in"):
# fail because of reading the hidden files and get duplicate keys
conf["parameters"]

0 comments on commit a91ccdb

Please sign in to comment.