Enable/disable preview for all the datasets when publishing Kedro-Viz from CLI #1894

jitu5 · 2024-05-08T11:21:47Z

Description

Resolves 1870

Development notes

--include-preview flag added for Viz CLI:
By default, kedro viz deploy and kedro viz build will have preview disabled for all the datasets
Introduced a ClassVar is_all_previews_enabled for DataNodeMetadata class which controls the enable/disable functionality for all the datasets

QA notes

Run kedro viz build --include-preview to enable preview for all datasets
Run kedro viz deploy --include-preview to enable preview for all datasets in the deployed version

NOTE: As discussed with @rashidakanchwala and @stephkaiser , kedro viz run will not have a preview flag and preview will be set to True by default. If the users want to change this behavior, they need to do this from UI. Designs will be updated soon (cc: @stephkaiser )

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

Signed-off-by: Jitendra Gundaniya <[email protected]>

Signed-off-by: <>

Signed-off-by: Jitendra Gundaniya <[email protected]>

Signed-off-by: ravi-kumar-pilla <[email protected]>

…ature/disable-preview Signed-off-by: ravi-kumar-pilla <[email protected]>

Signed-off-by: ravi-kumar-pilla <[email protected]>

astrojuanlu

Should we make a mention in the documentation?

rashidakanchwala · 2024-05-17T15:38:38Z

There's also this ticket here #1705 , we can add this to next sprint.

Signed-off-by: Jitendra Gundaniya <[email protected]>

merelcht

The code looks good, but I have some reservations about the naming of the new are_datasets_previewable boolean. It's getting confusing with the is_preview_disabled functionality we have.

merelcht · 2024-05-21T10:28:11Z

package/kedro_viz/api/rest/responses.py

@@ -452,7 +457,7 @@ def save_api_pipeline_response_to_fs(pipelines_path: str, remote_fs: Any):
            raise exc


-def save_api_responses_to_fs(path: str, remote_fs: Any):
+def save_api_responses_to_fs(path: str, remote_fs: Any, are_datasets_previewable: bool):


This is personal preference, but I would change are_datasets_previewable to preview_datasets.

To me are_datasets_previewable sounds like it's checking whether it's possible to preview these datasets or not, where when reading the code it should mean whether preview has been enabled or not.

In my proposed change preview_datasets=True means preview is enabled and preview_datasets=False means it's disabled.

okay, I too had a hard time renaming this :) .... so I want to make it differentiable to the is_preview_disabled which corresponds to each instance of DataNodeMetadata class, where-as the plural form are_datasets_previewable is a class variable. I personally prefer having is/are before a boolean variable/method to be clear of the return/data type.

merelcht · 2024-05-21T10:33:38Z

package/kedro_viz/models/flowchart.py

+            cls.data_node.is_preview_disabled()
+            or not hasattr(cls.dataset, "preview")
+            or not cls.are_datasets_previewable


What's the difference here between cls.data_node.is_preview_disabled() and cls.are_datasets_previewable? Semantically, it sounds very similar, so I think we should think carefully about how these flags are named so it's easy to understand what's happening for future team members.

Also in terms of functionality, what does it mean for preview to be disabled on a node level vs class level?

Agree with @merelcht.

i am also thinking for clarity we could possibly have :-

is_preview_disabled on node level (and explain this in the docstring, that this disabled via node level happens when the user specifies 'preview=false' in the DataCatalog)

is_all_previews_disabled on class level (and once again explain in docstring this is to disable previews for all nodes and can be done via CLI and UI)

@merelcht As discussed with @rashidakanchwala the existing is_preview_disabled will rename to is_preview_enabled and new boolean variable are_datasets_previewable to is_all_previews_enabled .

merelcht · 2024-05-21T10:35:53Z

One thought is perhaps to name the new boolean after the new --include-preview flag, so it's clear those two are connected?

Signed-off-by: Jitendra Gundaniya <[email protected]>

rashidakanchwala

LGTM. Can we please also create a a ticket/PR to change is_preview_disabled to is_preview_enabled so it is consistent.

thanks both @jitu5 and @ravi-kumar-pilla :D

jitu5 · 2024-05-22T10:17:42Z

@rashidakanchwala Sure I will create a separate PR for change of is_preview_disabled to is_preview_enabled

ravi-kumar-pilla

LGTM. Thank you

jitu5 · 2024-05-31T11:34:32Z

@stephkaiser Requesting you approval on disable preview from CLI, few days back, I demoed it you. please let me know if you have any questions.

stephkaiser · 2024-05-31T11:45:14Z

@stephkaiser Requesting you approval on disable preview from CLI, few days back, I demoed it you. please let me know if you have any questions.

no questions from me! looks good, thanks Jitendra!

# Conflicts: # package/kedro_viz/models/flowchart.py

jitu5 added 5 commits May 3, 2024 13:57

Add options for cli common with disable preview of datasets

74e0371

Signed-off-by: Jitendra Gundaniya <[email protected]>

New approach for disable preview

5398a11

Signed-off-by: <>

Disable preview for cli and lint fix

4da365f

Signed-off-by: Jitendra Gundaniya <[email protected]>

deploy api with preview

9f2afa2

Signed-off-by: Jitendra Gundaniya <[email protected]>

Removing UI changes

35f1436

Signed-off-by: Jitendra Gundaniya <[email protected]>

jitu5 self-assigned this May 8, 2024

jitu5 and others added 11 commits May 8, 2024 14:47

Removing UI changes

85e1a2d

Signed-off-by: Jitendra Gundaniya <[email protected]>

refactor disable preview cli

d8ef988

Signed-off-by: ravi-kumar-pilla <[email protected]>

fix lint

3377832

Signed-off-by: ravi-kumar-pilla <[email protected]>

fix lint

7a31729

Signed-off-by: ravi-kumar-pilla <[email protected]>

Merge branch 'main' of https://github.com/kedro-org/kedro-viz into fe…

028a7f3

…ature/disable-preview Signed-off-by: ravi-kumar-pilla <[email protected]>

testing 1

78f44bb

Signed-off-by: ravi-kumar-pilla <[email protected]>

change help text

663924c

Signed-off-by: ravi-kumar-pilla <[email protected]>

change help text

defbf66

Signed-off-by: ravi-kumar-pilla <[email protected]>

update tests

9195150

Signed-off-by: ravi-kumar-pilla <[email protected]>

format help text

c4c7375

Signed-off-by: ravi-kumar-pilla <[email protected]>

change default preview for run

6eaaaeb

Signed-off-by: ravi-kumar-pilla <[email protected]>

ravi-kumar-pilla changed the title ~~Enable users to disable preview for all when publishing Kedro-Viz~~ Enable/disable preview for all the datasets when publishing Kedro-Viz via CLI May 16, 2024

update release note

8372ec3

Signed-off-by: ravi-kumar-pilla <[email protected]>

ravi-kumar-pilla marked this pull request as ready for review May 16, 2024 23:00

ravi-kumar-pilla requested review from rashidakanchwala, astrojuanlu and ravi-kumar-pilla as code owners May 16, 2024 23:00

ravi-kumar-pilla requested review from stephkaiser and merelcht and removed request for ravi-kumar-pilla May 16, 2024 23:01

address vale suggestion

2f67e78

Signed-off-by: ravi-kumar-pilla <[email protected]>

ravi-kumar-pilla changed the title ~~Enable/disable preview for all the datasets when publishing Kedro-Viz via CLI~~ Enable/disable preview for all the datasets when publishing Kedro-Viz with CLI May 17, 2024

ravi-kumar-pilla added 2 commits May 16, 2024 23:01

fix pytests and linter

12eabd1

Signed-off-by: ravi-kumar-pilla <[email protected]>

adjust file permission

57b68b3

Signed-off-by: ravi-kumar-pilla <[email protected]>

stephkaiser approved these changes May 17, 2024

View reviewed changes

astrojuanlu reviewed May 17, 2024

View reviewed changes

jitu5 and others added 2 commits May 20, 2024 19:10

--include-preview flag added for CLI

c815a9b

Signed-off-by: Jitendra Gundaniya <[email protected]>

Merge branch 'main' into feature/disable-preview

5a43fda

merelcht reviewed May 21, 2024

View reviewed changes

Merge branch 'main' into feature/disable-preview

0998f7a

jitu5 mentioned this pull request May 21, 2024

Fix broken URL when active pipeline name changes on initial load #1914

Merged

5 tasks

jitu5 added 3 commits May 21, 2024 18:53

are_datasets_previewable renamed to is_all_previews_enabled

d06f36e

Signed-off-by: Jitendra Gundaniya <[email protected]>

lint fix

73b1944

Signed-off-by: Jitendra Gundaniya <[email protected]>

type ignore add for TRANSCODING_SEPARATOR

94e3501

Signed-off-by: Jitendra Gundaniya <[email protected]>

rashidakanchwala approved these changes May 22, 2024

View reviewed changes

Merge branch 'main' into feature/disable-preview

46dbf28

jitu5 mentioned this pull request May 23, 2024

Changed is_preview_disabled to is_preview_enabled #1927

Merged

5 tasks

Merge branch 'main' into feature/disable-preview

20bbe47

ravi-kumar-pilla self-requested a review May 29, 2024 18:17

ravi-kumar-pilla approved these changes May 29, 2024

View reviewed changes

jitu5 added 2 commits May 30, 2024 12:14

Merge branch 'main' into feature/disable-preview

b80c799

Merge branch 'main' into feature/disable-preview

788a491

stephkaiser approved these changes May 31, 2024

View reviewed changes

jitu5 closed this in #1927 May 31, 2024

jitu5 reopened this May 31, 2024

Merge branch 'main' into feature/disable-preview

57aec4e

# Conflicts: # package/kedro_viz/models/flowchart.py

jitu5 merged commit 8fe5fa4 into main May 31, 2024
39 checks passed

jitu5 deleted the feature/disable-preview branch May 31, 2024 12:59

SajidAlamQB mentioned this pull request Jul 25, 2024

Release v9.2.0 #2001

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable/disable preview for all the datasets when publishing Kedro-Viz from CLI #1894

Enable/disable preview for all the datasets when publishing Kedro-Viz from CLI #1894

jitu5 commented May 8, 2024 •

edited by ravi-kumar-pilla

Loading

astrojuanlu left a comment

rashidakanchwala commented May 17, 2024 •

edited

Loading

merelcht left a comment

merelcht May 21, 2024

ravi-kumar-pilla May 21, 2024

merelcht May 21, 2024

rashidakanchwala May 21, 2024 •

edited

Loading

jitu5 May 21, 2024

merelcht commented May 21, 2024

rashidakanchwala left a comment

jitu5 commented May 22, 2024

ravi-kumar-pilla left a comment

jitu5 commented May 31, 2024

stephkaiser commented May 31, 2024

Enable/disable preview for all the datasets when publishing Kedro-Viz from CLI #1894

Enable/disable preview for all the datasets when publishing Kedro-Viz from CLI #1894

Conversation

jitu5 commented May 8, 2024 • edited by ravi-kumar-pilla Loading

Description

Development notes

QA notes

Checklist

astrojuanlu left a comment

Choose a reason for hiding this comment

rashidakanchwala commented May 17, 2024 • edited Loading

merelcht left a comment

Choose a reason for hiding this comment

merelcht May 21, 2024

Choose a reason for hiding this comment

ravi-kumar-pilla May 21, 2024

Choose a reason for hiding this comment

merelcht May 21, 2024

Choose a reason for hiding this comment

rashidakanchwala May 21, 2024 • edited Loading

Choose a reason for hiding this comment

jitu5 May 21, 2024

Choose a reason for hiding this comment

merelcht commented May 21, 2024

rashidakanchwala left a comment

Choose a reason for hiding this comment

jitu5 commented May 22, 2024

ravi-kumar-pilla left a comment

Choose a reason for hiding this comment

jitu5 commented May 31, 2024

stephkaiser commented May 31, 2024

jitu5 commented May 8, 2024 •

edited by ravi-kumar-pilla

Loading

rashidakanchwala commented May 17, 2024 •

edited

Loading

rashidakanchwala May 21, 2024 •

edited

Loading