-
Notifications
You must be signed in to change notification settings - Fork 115
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
Docs for kedro viz build
#1710
Docs for kedro viz build
#1710
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍 . I have added some suggestions. Thank you
Signed-off-by: ravi-kumar-pilla <[email protected]>
… into add-build-docs Signed-off-by: ravi-kumar-pilla <[email protected]>
Co-authored-by: Ravi Kumar Pilla <[email protected]>
Co-authored-by: Merel Theisen <[email protected]>
Signed-off-by: Jo Stichbury <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy with this now @rashidakanchwala
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few comments
docs/source/share_kedro_viz.md
Outdated
|
||
Kedro-Viz requires specific minimum versions of `fsspec[s3]`, and `kedro` to publish your project. | ||
You can automate the process of publishing and sharing your Kedro-Viz. This section describes the steps for AWS, which is the only cloud provider supported for automation at present. Integration with other cloud providers, namely Azure and GCP, will be added soon. In the absence of automated publish and share for other platforms, there is a [manual, platform-agnostic publish and share process](#platform-agnostic-sharing-with-kedro-viz) described below. You can use the manual process for sharing on static website hosts like GitHub pages, and cloud providers like Azure and GCP. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: use line breaks to make code review easier (otherwise everything is just one very wide line)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This paragraph is a bit dense. Maybe it could be said "there are two ways to publish and share your Kedro-Viz: 1. the automatic way (only AWS is supported at present) and 2. the manual way (you can use any static hosting or object storage of your choosing)"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also: "share your Kedro-Viz" should be replaced by "share your Kedro-Viz project", right? I recall there was a discussion around terminology a while ago cc @stichbury
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's a valid comment but equally is fine as it is (partly because I wrote it) so happy for Rashida to merge as is or make some further changes, depends on urgency to get this release out.
docs/source/share_kedro_viz.md
Outdated
|
||
Follow the steps [listed in the GitHub pages documentation](https://docs.github.com/en/pages/quickstart) to create a Git repository that supports GitHub Pages. On completion, push the contents of the `build` folder to this new repository. Your site will be available at the following URL: `http://<username>.github.io` | ||
|
||
### Cloud providers such as Azure, and GCP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Nit] Can we include AWS as well and then at the end say For AWS, we also offer automated deployment ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For instance, we recently had a situation where someone was deploying to AWS. However, they encountered an issue because they had CloudFront enabled and were using their own DNS. As a result, they were unable to use the direct deploy method.
It's too early to say but I think direct deploy might cause issues. kedro build
way might be the more robust chosen approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a Nit comment but overall the doc looks good to me 💯 . Thank you
Description
Development notes
QA notes
Checklist
RELEASE.md
file