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

Change 'Kedro Viz' to 'Kedro Viz Run' #3430

Merged
merged 11 commits into from
Dec 19, 2023
Merged

Conversation

MehdiNV
Copy link
Contributor

@MehdiNV MehdiNV commented Dec 14, 2023

Description

Resolves 1616 and acts as a follow-up to PR 1671.

This PR updates references of 'kedro viz' to be the new correct 'kedro viz run' instead.

Development notes

Adjusts docs, simple textual changes

QA notes

Can be tested normally via npm test, the e2e tests, and directly using the command group in the CLI; should have no bearing on existing tests though, as changes are solely docs-specific

Signed-off-by: mehdinv <mehdinv@hotmail.com>
Signed-off-by: mehdinv <mehdinv@hotmail.com>
# Conflicts:
#	docs/source/experiment_tracking/index.md
#	docs/source/extend_kedro/common_use_cases.md
#	docs/source/get_started/new_project.md
#	docs/source/visualisation/kedro-viz_visualisation.md
#	docs/source/visualisation/preview_datasets.md
#	docs/source/visualisation/visualise_charts_with_plotly.md
Signed-off-by: Mehdi Naderi Varandi <MehdiNV@hotmail.com>
Signed-off-by: Mehdi Naderi Varandi <MehdiNV@hotmail.com>
Copy link
Member

@merelcht merelcht left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

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
Contributor

@stichbury stichbury left a comment

Choose a reason for hiding this comment

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

This looks good to me. I'm not seeing the same updates in the viz docs though -- does this mean that either (a) they need to be done or (b) they'll be dropped in only when they apply and currently kedro viz is the command to use and not kedro viz run.

@astrojuanlu
Copy link
Member

Docs failure was transient:

WARNING: mermaid code 'flowchart TD\n  A[Start] --> B{Do you prefer developing your projects in notebooks?}\n  B -->|Yes| C[Use a Databricks workspace to develop a Kedro project]\n  B -->|No| D{Are you a beginner with Kedro?}\n  D -->|Yes| E[Use an IDE, dbx and Databricks Repos to develop a Kedro project]\n  D -->|No| F{Do you have advanced project requirements<br>e.g. CI/CD, scheduling, production-ready, complex pipelines, etc.?}\n  F -->|Yes| G{Is rapid development needed for your project needs?}\n  F -->|No| H[Use an IDE, dbx and Databricks Repos to develop a Kedro project]\n  G -->|Yes| I[Use an IDE, dbx and Databricks Repos to develop a Kedro project]\n  G -->|No| J[Use a Databricks job to deploy a Kedro project]': Mermaid exited with error:
[stderr]
b'\nTimeoutError: Timed out after 30000 ms while waiting for the WS endpoint URL to appear in stdout!\n    at ChromeLauncher.launch (file:///home/docs/.asdf/installs/nodejs/19.0.1/lib/node_modules/@mermaid-js/mermaid-cli/node_modules/puppeteer-core/lib/esm/puppeteer/node/ProductLauncher.js:119:23)\n    at async run (file:///home/docs/.asdf/installs/nodejs/19.0.1/lib/node_modules/@mermaid-js/mermaid-cli/src/index.js:404:19)\n    at async cli (file:///home/docs/.asdf/installs/nodejs/19.0.1/lib/node_modules/@mermaid-js/mermaid-cli/src/index.js:184:3)\n\n'
[stdout]
b''

See #3395

MehdiNV and others added 2 commits December 19, 2023 02:07
Co-authored-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Mehdi Naderi Varandi <MehdiNV@hotmail.com>
@MehdiNV
Copy link
Contributor Author

MehdiNV commented Dec 19, 2023

Docs failure was transient:

WARNING: mermaid code 'flowchart TD\n  A[Start] --> B{Do you prefer developing your projects in notebooks?}\n  B -->|Yes| C[Use a Databricks workspace to develop a Kedro project]\n  B -->|No| D{Are you a beginner with Kedro?}\n  D -->|Yes| E[Use an IDE, dbx and Databricks Repos to develop a Kedro project]\n  D -->|No| F{Do you have advanced project requirements<br>e.g. CI/CD, scheduling, production-ready, complex pipelines, etc.?}\n  F -->|Yes| G{Is rapid development needed for your project needs?}\n  F -->|No| H[Use an IDE, dbx and Databricks Repos to develop a Kedro project]\n  G -->|Yes| I[Use an IDE, dbx and Databricks Repos to develop a Kedro project]\n  G -->|No| J[Use a Databricks job to deploy a Kedro project]': Mermaid exited with error:
[stderr]
b'\nTimeoutError: Timed out after 30000 ms while waiting for the WS endpoint URL to appear in stdout!\n    at ChromeLauncher.launch (file:///home/docs/.asdf/installs/nodejs/19.0.1/lib/node_modules/@mermaid-js/mermaid-cli/node_modules/puppeteer-core/lib/esm/puppeteer/node/ProductLauncher.js:119:23)\n    at async run (file:///home/docs/.asdf/installs/nodejs/19.0.1/lib/node_modules/@mermaid-js/mermaid-cli/src/index.js:404:19)\n    at async cli (file:///home/docs/.asdf/installs/nodejs/19.0.1/lib/node_modules/@mermaid-js/mermaid-cli/src/index.js:184:3)\n\n'
[stdout]
b''

See #3395

Ah makes a lot of sense; was completely lost by what this error was

@MehdiNV
Copy link
Contributor Author

MehdiNV commented Dec 19, 2023

This looks good to me. I'm not seeing the same updates in the viz docs though -- does this mean that either (a) they need to be done or (b) they'll be dropped in only when they apply and currently kedro viz is the command to use and not kedro viz run.

It'll be A yeah, they'll need to be done good catch - I'll apply this over there as well ASAP

@MehdiNV MehdiNV merged commit c30b94b into main Dec 19, 2023
11 checks passed
@MehdiNV MehdiNV deleted the chore/update-viz-cli-command branch December 19, 2023 13:00
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