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

Update get_kedro_project_json_data import path #152

Merged
merged 2 commits into from
Nov 22, 2024
Merged

Conversation

jitu5
Copy link
Contributor

@jitu5 jitu5 commented Nov 14, 2024

Descriptions

Resolves #143

This pull request includes a change to the bundled/tool/lsp_server.py file to improve compatibility with different versions of kedro-viz. The most important change is the addition of a try-except block to handle imports for kedro-viz versions greater than and equal to 10.0.0.

Developer Note

  • bundled/tool/lsp_server.py: Added a try-except block to import get_kedro_project_json_data from the correct module based on the kedro-viz version.

@jitu5 jitu5 self-assigned this Nov 14, 2024
@jitu5 jitu5 added Kedro-Viz Functionality/Fix needs to done from Kedro-Viz dependencies Pull requests that update a dependency file labels Nov 14, 2024
@jitu5 jitu5 marked this pull request as ready for review November 14, 2024 12:17
Copy link
Contributor

@noklam noklam left a comment

Choose a reason for hiding this comment

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

Is this still needed after #155? Would you bump the minimum version of kedro-viz to 10.1.0 after this?

@jitu5
Copy link
Contributor Author

jitu5 commented Nov 21, 2024

@noklam

Is this still needed after #155?

Yes, as #155 updates the NPM and not pypi, Also we need to update this from kedro_viz.api.rest.responses.pipelines import get_kedro_project_json_data as get_kedro_project_json_data moved to kedro_viz.api.rest.responses.pipelines

Would you bump the minimum version of kedro-viz to 10.1.0 after this?

I will update the minimum version of kedro-viz to 10.1.0 in this PR only

@rashidakanchwala
Copy link

I suppose we will have to be careful abt making such changes on kedro-viz in the future as it is a breaking change on vscode.

@noklam
Copy link
Contributor

noklam commented Nov 22, 2024

@rashidakanchwala It is technically a breaking change since it's public API. It's ok to do the refactor but you may need to leave the import in the original place temporarily to keep it non-breaking until 11.0.0. i.e. keep this in kedro_viz.api.rest.responses.py

from .pipelines import get_kedro_project_json_data

@noklam noklam merged commit e49cfa5 into main Nov 22, 2024
2 checks passed
@noklam noklam deleted the fix/support-latest-viz branch November 22, 2024 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file Kedro-Viz Functionality/Fix needs to done from Kedro-Viz
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update import path for get_kedro_project_json_data
4 participants