-
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
Migrate Demo from AWS Lightsail to GitPages #2034
Conversation
@@ -24,7 +24,7 @@ cypress/screenshots | |||
cypress/downloads/ | |||
|
|||
# production | |||
build/ | |||
/build/ |
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.
build seems to be in the root similar to lib. Why is this needed ?
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.
we want to ignore the root build but keep the demo/build. build folder is created when you do npm run build
and we don't want to accidentally push that to github.
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.
yes, I understand that. Before this change we were doing the same right. The forward slash might not be needed I guess
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.
when i remove the forward slash, it also ignores the demo/build which we don't want to ignore anymore as it has static graphql files
@rashidakanchwala In this PR we are adding all the required files in demo-project/build but what happens for new release? will it overwrite existing build folder ? |
@jitu5 , In the current build folder, I’ve included only the GraphQL files and the .api/packages-compatibilities file. I tested the setup, and during the |
if (networkError) { | ||
console.error(`[Network error]: ${networkError}`); | ||
|
||
// Try reading from static file if network error occurs |
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.
Instead of checking for network error and then reading from a static file path, can we check for the host url ? Like if the url host is the demo site, then fetch from file instead of making a network call ? This seems like a round-trip which is not needed if we already know the request is made from demo site
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.
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 am thinking doing this but first I want to test it out with demo.kedro.org. So I will make this change in a follow up PR where I update the github workflow to 'public-kedro-viz' actions
Co-authored-by: Ravi Kumar Pilla <ravi_kumar_pilla@mckinsey.com> Signed-off-by: rashidakanchwala <37628668+rashidakanchwala@users.noreply.github.com>
Co-authored-by: Ravi Kumar Pilla <ravi_kumar_pilla@mckinsey.com> Signed-off-by: rashidakanchwala <37628668+rashidakanchwala@users.noreply.github.com>
Co-authored-by: Ravi Kumar Pilla <ravi_kumar_pilla@mckinsey.com> Signed-off-by: rashidakanchwala <37628668+rashidakanchwala@users.noreply.github.com>
Co-authored-by: Ravi Kumar Pilla <ravi_kumar_pilla@mckinsey.com> Signed-off-by: rashidakanchwala <37628668+rashidakanchwala@users.noreply.github.com>
Description
Migration of Demo Project from AWS to GitHub Pages
Rationale for Migration:
Challenges with Migration:
We couldn't migrate earlier because static hosting worked for the flowchart but not for experiment tracking.
Updates in This PR:
Next Steps:
Eventually, we will transition to using
publish-share-kedro-viz
action. For now, until we release the above changes, this method will allow our demo site to function. We plan to testpublish-share-kedro-viz
action on a test repo after the release to ensure it works as expected.Once this PR is approved, I will merge it to main and push it to the demo branch, triggering the 'new' deploy-demo workflow. Afterward, I will update demo.kedro.org to point to the GitHub Pages site.
Test link - https://kedro-org.github.io/kedro-viz/?types=nodes,datasets&pid=__default__&expandAllPipelines=false
Checklist
RELEASE.md
file