Closed ankatiyar closed 1 month ago
[Question for reviewers]: We should probably error out if the user has more than one catch-all patterns in this case, does that make sense?
Yes
Just dumping my thought here in case I am blocking the PRs. I have some questions about the shallow_copy
approach, but that was done long before this PR so I don't want to expand the discussion too much.
I think:
DataCatalog
is initialised. (I think this is consistent as the shallow_copy
will create a new DataCatalog
, this is where I find the shallow_copy
weird as it does not seem to be a reference but completely new objectThe most important question here is "should runner dataset patterns override DataCatalog
"? In framework setting, the way to override patterns is catalog.yml
. However, the order of how framework work is different.
DataCatalog
read catalog.yml
first This suggests that one should only use either catalog.yml
or runner
. Should it fails or just ignore the dataset pattern from runner? For me, I think I will go for raising an error instead of ignoring the pattern sliently.
Description
Fix #3720
Development notes
TODO:
Wanted to get opinions on the proposed solution before proceeding with the pending tasks mentioned above^
Proposed Solution
When
DataCatalog
is created (infrom_config
)DataCatalog._default_pattern
(This would be the user-defined default pattern, not from the runner)When a dataset is being fetched
DataCatalog._dataset_patterns
ORDataCatalog._default_pattern
_dataset_patterns
OR_default_pattern
When runner.run() is called
When
kedro run
is executed, the runner creates a shallow copy of the catalog object by calling theDataCatalog.shallow_copy()
extra_dataset_patterns = {'{default}': MemoryDataset}
_default_pattern
- it's not needed to do anythingDataCatalog._dataset_patterns
(this will then not emit a warning about the catch-all pattern being used over the runner default dataset)For the catalog CLI commands
Similar logic as above has been copied for the matching of datasets to patterns
Checklist
RELEASE.md
file