Introduce --include-hooks option and remove --ignore-plugins in Kedro-Viz

ravi-kumar-pilla commented 3 months ago

Description

Resolves #1808

Development notes

Added --include-hooks option to all the viz cli commands (run, build, deploy)
Added --include-hooks option to %run_viz Jupyter line magic
Removed --ignore-plugins option from viz cli commands
Updated pytests

QA notes

All tests should pass
--include-hooks should appear as an option with -h (eg., kedro viz run -h)

Checklist

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

ravi-kumar-pilla commented 3 months ago

CircleCI Build fix - https://github.com/kedro-org/kedro-viz/pull/1819

brau0300 commented 2 months ago

This (as expected it seems) was a breaking change for any project using hooks or plugins and was non-trivial to chase down.

It also makes it less convenient for especially new users on a project needing this flag to start kedro viz; would we be open to considering ways to pass this option (now required for certain projects) by default (e.g. via project configuration) and/or more succinctly?

ravi-kumar-pilla commented 2 months ago

This (as expected it seems) was a breaking change for any project using hooks or plugins and was non-trivial to chase down.

It also makes it less convenient for especially new users on a project needing this flag to start kedro viz; would we be open to considering ways to pass this option (now required for certain projects) by default (e.g. via project configuration) and/or more succinctly?

Hi @brau0300 ,

Thank you for the comment. Ideally Kedro-Viz does not need any hook to start. Users should be able to do kedro viz run without any issue. Having said that, could you please let us know the use case where you need a hook to be enabled for kedro viz to start ?

At this moment, we do not have any discussions on making this available via configuration of some sort. But we would be happy to hear from you the pain points and the issue this would solve for Kedro-Viz. Thank you

brau0300 commented 1 month ago

Apologies for the delayed response. We are using some kedro starters we've developed to help medium-maturity users get going faster on projects. We are using DataCatalog hooks in several projects and also kedro boot in another (another framework that uses plugins/hooks) https://github.com/takikadiri/kedro-boot ; in both cases the users now see scary warning messages when they run kedro viz with kedro viz 9 because the hooks are needed to resolve parameters/datasets at runtime. We have updated our readmes to use the new --include-hooks flag, but definitely folks will now need to learn the new way of starting viz - not really a big deal for folks who are comfortable/developing every day, but definitely more words to type and different from how things were. Hopefully that helps provide some context - not sure if there is a great solution, but hopefully it's helpful to share an example of how the change in default behavior gets perceived by users.

One other thing to mention as a wrinkle - we had our starters pinned to kedro viz 8, but mainline kedro dropping toposort now causes problems with newer versions of kedro mixed with kedro viz 8. We're tentatively thinking we keep kedro viz unpinned going forward, and have updated our instructions for new users to use the --include-hooks flag

On Wed, May 1, 2024 at 3:32 PM Ravi Kumar Pilla @.***> wrote:

This (as expected it seems) was a breaking change for any project using hooks or plugins and was non-trivial to chase down.

It also makes it less convenient for especially new users on a project needing this flag to start kedro viz; would we be open to considering ways to pass this option (now required for certain projects) by default (e.g. via project configuration) and/or more succinctly?

Hi @brau0300 https://github.com/brau0300 ,

Thank you for the comment. Ideally Kedro-Viz does not need any hook to start. Users should be able to do kedro viz run without any issue. Having said that, could you please let us know the use case where you need a hook to be enabled for kedro viz to start ?

At this moment, we do not have any discussions on making this available via configuration of some sort. But we would be happy to hear from you the pain points and the issue this would solve for Kedro-Viz. Thank you

— Reply to this email directly, view it on GitHub https://github.com/kedro-org/kedro-viz/pull/1818#issuecomment-2089086426, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAD2Y5QZW67TKVNGSXRNBGLZAFGMBAVCNFSM6AAAAABFJJMKSKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBZGA4DMNBSGY . You are receiving this because you were mentioned.Message ID: @.***>

ravi-kumar-pilla commented 1 month ago

Hi @brau0300 ,

Thank you for the response. The idea behind ignoring hooks by default was to make Kedro-Viz faster and work towards https://github.com/kedro-org/kedro-viz/issues/1159 . I agree it is a breaking change, but the team decided to load Kedro-Viz with bare minimum of what it needs and users can opt in hooks if they want any additional functionality.

For the issue of dropping toposort, this could have been avoided if Kedro-Viz refrained from using transitive dependencies. We have created a ticket to make Kedro-Viz better and avoid any such instances in future.

Thank you

kedro-org / kedro-viz