-
Notifications
You must be signed in to change notification settings - Fork 113
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
Make Kedro-Viz shareable via a hosted URL #1487
Conversation
Signed-off-by: Rashida Kanchwala <[email protected]>
Signed-off-by: Rashida Kanchwala <[email protected]>
…dro-viz into kedro-viz-save-load Signed-off-by: Rashida Kanchwala <[email protected]>
Signed-off-by: Rashida Kanchwala <[email protected]>
Signed-off-by: Rashida Kanchwala <[email protected]>
Signed-off-by: Rashida Kanchwala <[email protected]>
Signed-off-by: Tynan DeBold <[email protected]>
Signed-off-by: Tynan DeBold <[email protected]>
Signed-off-by: Tynan DeBold <[email protected]>
Signed-off-by: Tynan DeBold <[email protected]>
Signed-off-by: Tynan DeBold <[email protected]>
…ature/shareable-urls Signed-off-by: Tynan DeBold <[email protected]>
Signed-off-by: Tynan DeBold <[email protected]>
…ove WS/subscription code Signed-off-by: Tynan DeBold <[email protected]>
Signed-off-by: Tynan DeBold <[email protected]>
Signed-off-by: Tynan DeBold <[email protected]>
…reable-viz-backend
Signed-off-by: Tynan DeBold <[email protected]>
Signed-off-by: Tynan DeBold <[email protected]>
…/kedro-viz into feature/shareable-urls Signed-off-by: Tynan DeBold <[email protected]>
…ature/shareable-urls Signed-off-by: Tynan DeBold <[email protected]>
…kedro-viz into feature/shareable-urls Signed-off-by: Tynan DeBold <[email protected]>
Signed-off-by: Tynan DeBold <[email protected]>
Signed-off-by: Tynan DeBold <[email protected]>
Signed-off-by: Tynan DeBold <[email protected]>
…/kedro-viz into feature/shareable-urls Signed-off-by: Tynan DeBold <[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 have made few suggestions, none of them is a blocking change. Looks great !
* fix telemetry for sharaeable viz Signed-off-by: ravi-kumar-pilla <[email protected]> * fix lint issue Signed-off-by: ravi-kumar-pilla <[email protected]> --------- Signed-off-by: ravi-kumar-pilla <[email protected]> Signed-off-by: Tynan DeBold <[email protected]> Co-authored-by: Tynan DeBold <[email protected]>
src/apollo/subscriptions.js
Outdated
import gql from 'graphql-tag'; | ||
|
||
/** subscribe to receive new runs */ | ||
export const NEW_RUN_SUBSCRIPTION = gql` |
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.
@tynandebold - Since we are removing this subscription feature from Experiment Tracking I think it should have it's own PR. So in the future, incase we want to add it back or we want to explore this again, there's history. Especially because based on this subscription feature we were planning other features like auto-reload on Collab Experiment Tracking and so on.
Right now because it's part of Shareable-viz -- it is not obvious.
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.
Fair enough.
Please review this today, if you can: #1554
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 have gone through the code and suggested some changes.
Signed-off-by: Tynan DeBold <[email protected]>
…/kedro-viz into feature/shareable-urls Signed-off-by: Tynan DeBold <[email protected]>
Signed-off-by: Tynan DeBold <[email protected]>
Signed-off-by: Tynan DeBold <[email protected]>
Signed-off-by: Tynan DeBold <[email protected]>
@@ -15,7 +16,10 @@ import './feature-hints.scss'; | |||
|
|||
const localStorageKeyShowHints = 'showFeatureHints'; | |||
export const localStorageKeyFeatureHintsStep = 'featureHintStep'; | |||
const numFeatureHints = featureHintsContent.length; | |||
const updatedFeatureHintsContent = isRunningLocally() |
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 is not a blocker but just for clarity-- from what I understand 'Publish and share Kedro-viz' feature hint shouldn't show in the deployed version and hence we have to write this code.
But this feature hint only gets shown when the user hovers over the 'deploy button' and there won't be any deploy button in the hosted version, so why do we still need to have this check here?
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.
Because even if the indicator dot isn't there the content in the card would still exist.
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.
Thanks for this!
…ature/shareable-urls Signed-off-by: Tynan DeBold <[email protected]>
Signed-off-by: Tynan DeBold <[email protected]>
Description
Closes #1116
Development notes
This PR enables Viz to be published and hosted on AWS S3.
For the frontend, most changes exist in
shareable-url-modal.js
. On the deployed version, we show a timestamp for context, which comes from theshareable-url-metadata.js
file.A new feature hint for the Sharable URLs button is added in
feature-hints-content.js
. I've also updatedfeature-hints.js
to remove that feature hint from the deployed version, since it doesn't make sense for the user to see it there.QA notes
Important
You don't need to review any Python files in this PR (nothing inside
/package
)!!! That review will be done in #1498.To get setup to test this locally:
fsspec
version2023.9.0
or greater.kedro
version18.2.0
or greater.make run
.npm start
.Checklist
RELEASE.md
file