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

Support Kedro 0.19 #165

Draft
wants to merge 11 commits into
base: develop
Choose a base branch
from

Conversation

sfczekalski
Copy link

No description provided.

@sfczekalski
Copy link
Author

I don't see an option to comment on the code that hasn't been changed, so I'm writing it down here:

I have doubts how the test_runner_fills_missing_datasets test should work - the nodes are run one by one as independent pipelines, and the only dataset which is added to the catalog is the input_data.

When the first node runs, it loads this dataset, but it doesn't save any output.

The second node should read the first node's output, i2, the input dataset gets created, but there is no data to load, as the fist node hasn't saved any result.

I thought that maybe we should add the output datasets of each node to unsatisfied list in the AzurePipelinesRunner, so that the first node will save its output to i2, but in the test, we also check the free results of the third node run, which is empty, if we add its output to the catalog.

Comment on lines +83 to +85
# os.chdir("tests")
yield patched_package
os.chdir(original_dir)
# os.chdir(original_dir)
Copy link
Author

Choose a reason for hiding this comment

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

Not sure why changing the directory used to be there

Comment on lines +23 to +24
for dataset_name in catalog.list():
dataset = catalog._get_dataset(dataset_name, suggest=False)

Choose a reason for hiding this comment

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

@ElenaKhaustova what would be the best way to do this with the new catalog?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants