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

Make Kedro-Viz compatible with previous Kedro with 'DataSet' #1445

Merged
merged 32 commits into from
Jul 12, 2023

Conversation

rashidakanchwala
Copy link
Contributor

@rashidakanchwala rashidakanchwala commented Jul 11, 2023

Description

In Kedro version 0.18.11, 'DataSet' was changed to 'Dataset' in several places. This change was also reflected in Kedro-viz to align with the latest Kedro version. However, this created compatibility issues for users who are using the latest Kedro-viz with a previous version of Kedro.

To address this problem and ensure Kedro-viz supports older versions of Kedro. In this PR, we handle the situation by attempting to import the newer 'Dataset' first. If an ImportError is raised, indicating that the newer version is not available, we catch the error and import the older 'DataSet' instead.

Development notes

QA notes

Checklist

  • Read the contributing guidelines
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added new entries to the RELEASE.md file
  • Added tests to cover my changes

Copy link
Member

@astrojuanlu astrojuanlu left a comment

Choose a reason for hiding this comment

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

This is the way 👍

Left a comment about another upcoming deprecation.

@deepyaman deepyaman changed the title Make Kedro-viz compatible with previous Kedro with 'DataSet' Make Kedro-Viz compatible with previous Kedro with 'DataSet' Jul 12, 2023
@deepyaman deepyaman force-pushed the make-kedro-viz-compatible-with-old-kedro branch from a3fc436 to e9e9d73 Compare July 12, 2023 07:59
@deepyaman
Copy link
Member

@rashidakanchwala @astrojuanlu I think #1447 should work (there's one lint error, I honestly am not seeing why off the top of my head), but it would require releasing Kedro with the type hint changes in kedro-org/kedro#2788. So, until it's released, will ignore the errors. I'll make the changes on this branch now to ignore.

@deepyaman deepyaman force-pushed the make-kedro-viz-compatible-with-old-kedro branch 2 times, most recently from 8d63d11 to 0f21fe7 Compare July 12, 2023 11:43
@rashidakanchwala
Copy link
Contributor Author

finally, thanks @deepyaman

@astrojuanlu and @merelcht -- can we pls review this!

Copy link
Member

@astrojuanlu astrojuanlu left a comment

Choose a reason for hiding this comment

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

👍🏽

Copy link
Member

@tynandebold tynandebold left a comment

Choose a reason for hiding this comment

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

💥

@deepyaman deepyaman merged commit 17d0a0b into main Jul 12, 2023
@deepyaman deepyaman deleted the make-kedro-viz-compatible-with-old-kedro branch July 12, 2023 12:47
@rashidakanchwala rashidakanchwala mentioned this pull request Jul 12, 2023
5 tasks
rashidakanchwala added a commit that referenced this pull request Jul 12, 2023
It's a patch release to fix Kedro-viz backward compatibility issue with Kedro and also fix compatibility with strawberry-graphql. Below are release notes :

Bump strawberry-graphql to at least version 0.192 to support the new
strawberry.union syntax. (Fix strawberry-graphql union error.  #1441)
Resolve the incompatibility between Kedro-Viz and Kedro versions prior to 0.18.11. (Make Kedro-Viz compatible with previous Kedro with 'DataSet'  #1445)
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.

4 participants