-
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
Fix kedro viz --lite
compatibility with Kedro 18
#2102
Fix kedro viz --lite
compatibility with Kedro 18
#2102
Conversation
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.
LGTM thanks @rashidakanchwala 👍
Thanks @rashidakanchwala ! My only question (not a blocker) - any clue why this wasn't caught by unit tests? |
excellent question and I am going to create an issue around it today. Ideally; this should be caught by e2e testing which we don't have any for Update - #2103 |
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 @rashidakanchwala for fixing the issue and creating the ticket. I did add one e2e test only against the "latest" Kedro version. I will add more in the ticket #2103 . Thank you ! |
Description
This is to get Kedro-viz (mainly kedro viz --lite) to work with Kedro 18 and some Kedro 19 versions based on changes this PR (kedro-org/kedro#3920) introduced.
In July, Kedro made the
_save
and_load
methods public. At that time, Kedro-viz did not rely on these methods. However, when we recently mergedkedro viz --lite
, we introduced an UnavailableDataset class, which is an AbstractDataset. This class now uses the public load and save methods. To maintain backward compatibility with older versions of the dataset, we followed a suggestion made by @deepyamanOriginally posted by @deepyaman in kedro-org/kedro#3920 (comment)
QA notes
Checklist
RELEASE.md
file