-
Notifications
You must be signed in to change notification settings - Fork 115
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
Enable lazy loading support for DataCatalog 2.0 on Kedro-Viz #2272
Enable lazy loading support for DataCatalog 2.0 on Kedro-Viz #2272
Conversation
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think with the new catalog we can significantly simplify overall logic, which will be the same for default
and lite
modes:
- We still need to resolve dataset factory patterns, but we do it via the
catalog.config_resolver.resolve_pattern()
method—it returns dataset configuration and does not require dataset initialization. - The above step is the only step needed to display the dataset
- Once the user clicks on the dataset, we get it from the catalog, then call preview and process failure if something is uninstalled
- That way, we don't really need
UnavailableDataset
andlite
mode becomesdefault.
I see that the difficulty is in supporting different catalogs simultaneously and fitting different concepts together. Maybe we need to discuss how to apply the above concept to further transition to a new catalog while maintaining backward compatibility.
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <90610031+SajidAlamQB@users.noreply.github.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to handle kedro viz --lite
correctly here. I think we are breaking it. Because when I try to uninstall kedro-datasets
and do kedro viz --lite
it breaks. This is not supposed to happen. kedro viz --lite
should work regardless if dependencies are there or not. If kedro-datasets
is missing, it runs and just makes all datasets UnavailableDatasets.
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
…//github.com/kedro-org/kedro-viz into feat/enable-lazy-loading-of-datasets-feature Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
|
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, now it makes sense.
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some questions about the forward compatibility of the KedroDataCatalog
kedro-org/kedro#4510 (comment) not sure they should block this PR, but I think it would be cool to sort them out ahead of the next releases.
The current implementation should work with Kedro 0.18.x through 1.0.x (assuming We could add version detection logic to handle the transition to Kedro 1.0+ differently. Would you recommend implementing that now, or waiting until Kedro 1.0 is closer to release? |
Good question, i thought Kedro 1.0 will deprecate old DataCatalog but I just saw the notes and it says it will be deprecated in 2.0. So far the plan was for Kedro-viz to remove the old DataCatalog methods when Kedro releases 1.0. cc- @ElenaKhaustova , @merelcht update - Spoke to Merel, this hasn't been decided yet. Anyways Kedro-viz plan was to continue to support old DataCatalog until Kedro 1.0 is released. So I suppose that decision doesn't block this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works well !! Thank you @SajidAlamQB
Description
Related to: #2213
This PR integrates the new lazy-loading feature from Kedro’s updated DataCatalog API into Kedro-Viz.
Specifically, it checks if the project’s catalog is an instance of
KedroDataCatalog
. This allows Kedro-Viz to show dataset details without requiring full dataset installation. It also prevents unintended data loads if a dataset is configured to be lazy.Development notes
KedroDataCatalog
with fallbacksOptional[AbstractDataset]
)QA notes
KedroDataCatalog
Checklist
RELEASE.md
file