Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update error message when catalog entry is invalid #3944

Merged
merged 4 commits into from
Jun 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions RELEASE.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Major features and improvements

## Bug fixes and other changes
* Updated error message for invalid catalog entries.

## Breaking changes to the API

Expand Down
8 changes: 6 additions & 2 deletions kedro/io/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ def from_config(
except Exception as exc:
raise DatasetError(
f"An exception occurred when parsing config "
f"for dataset '{name}':\n{str(exc)}"
f"for dataset '{name}':\n{str(exc)}."
) from exc

try:
Expand Down Expand Up @@ -384,7 +384,11 @@ def parse_dataset_definition(
config = copy.deepcopy(config)

if "type" not in config:
raise DatasetError("'type' is missing from dataset catalog configuration")
raise DatasetError(
"'type' is missing from dataset catalog configuration."
"\nHint: If this catalog entry is intended for variable interpolation, "
"make sure that the top level key is preceded by an underscore."
)

dataset_type = config.pop("type")
class_obj = None
Expand Down
7 changes: 7 additions & 0 deletions kedro/io/data_catalog.py
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,13 @@ class to be loaded is specified with the key ``type`` and their
user_default = {}

for ds_name, ds_config in catalog.items():
if not isinstance(ds_config, dict):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In #3555 there was a bit of discussion on what to do with values that are dict type, but not meant as dataset. Is it worth updating the message for that case as well to mention interpolation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I've added a hint to the error that comes from AbstractDataset.from_config() as well!

raise DatasetError(
f"Catalog entry '{ds_name}' is not a valid dataset configuration. "
"\nHint: If this catalog entry is intended for variable interpolation, "
"make sure that the key is preceded by an underscore."
)

ds_config = _resolve_credentials( # noqa: PLW2901
ds_config, credentials
)
Expand Down
10 changes: 10 additions & 0 deletions tests/io/test_data_catalog.py
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,16 @@ def test_config_invalid_arguments(self, sane_config):
with pytest.raises(DatasetError, match=pattern):
DataCatalog.from_config(**sane_config)

def test_config_invalid_dataset_config(self, sane_config):
sane_config["catalog"]["invalid_entry"] = "some string"
pattern = (
"Catalog entry 'invalid_entry' is not a valid dataset configuration. "
"\nHint: If this catalog entry is intended for variable interpolation, "
"make sure that the key is preceded by an underscore."
)
with pytest.raises(DatasetError, match=pattern):
DataCatalog.from_config(**sane_config)

def test_empty_config(self):
"""Test empty config"""
assert DataCatalog.from_config(None)
Expand Down