-
Notifications
You must be signed in to change notification settings - Fork 268
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
Add initial workflow for deploying the website to WP Cloud #1293
Conversation
@@ -0,0 +1,77 @@ | |||
name: Finish deploying to playground.wordpress.net (don't use manually) |
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.
Tweaked this name so it is not the same as the existing deployment workflow.
deploy_to_wp_cloud: | ||
# Only run this workflow from the trunk branch and when it's triggered by another workflow OR certain maintainers | ||
if: > | ||
github.ref == 'refs/heads/trunk' && ( |
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 thought we could identify this in the on:
section, and then the entire thing would only run on trunk
or on merges into trunk
.
with the if:
I thought that the job will still run but fail. maybe I'm remembering backwards
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.
with the if: I thought that the job will still run but fail. maybe I'm remembering backwards
@dmsnell it sounds like this will just skip the specific job if the condition is not satisfied:
https://docs.github.com/en/actions/using-jobs/using-conditions-to-control-job-execution#example-only-run-job-for-specific-repository
We also have a condition set on the playground-wordpress-net-wp-cloud
workflow environment that says it only runs on "Protected Branches", and in our case that means it only runs on "trunk".
I thought we could identify this in the on: section, and then the entire thing would only run on trunk or on merges into trunk.
It looks like it is also possible to filter on branch under the on:
, but we don't use that in deploy-website.yml which is what this new workflow was adapted from.
fi; | ||
echo "$API_HASH was not $GITHUB_SHA, waiting 10 seconds..."; | ||
sleep 10; | ||
done; |
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.
if this step using another CI job? can we reuse the workflow and avoid this waiting code?
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.
@dmsnell, this is indeed duplicated code from deploy-website.yml, but once we are done migrating to the new host, I imagine we will remove the original workflow and leave this as the only remaining "waiting block".
I'm not sure if there is a way to avoid the waiting entirely... Are you thinking the waiting code may not be required at all if we take another approach, or do you just mean that we could avoid duplicating it?
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 meant that maybe Github will schedule this Job once the previous one finishes, and we won't have to sit in a 10s wait loop. I have no idea if that's possible
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.
Ah, thanks. That's actually what this bit does:
https://github.com/WordPress/wordpress-playground/pull/1293/files#diff-e4927ad07277ff5004af1672f9a870dcf7c859ac71348aefd357ef9f30348d51R4-R8
A confusing thing is that the name within "build-website.yml" its name is "Deploy to playground.wordpress.net", which is the name the actual deploy workflow triggers on:
on:
workflow_run:
workflows: [Deploy to playground.wordpress.net]
types:
- completed
Apparently, based on the current deploy workflow, GH API still takes some time to reflect an artifact uploaded by the previous workflow, but I have not tested that assumption.
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.
The only reason building a website and deploying a website are separate workflows are GitHub limitations around downloading artifacts. Not going too deep into details, this PR is not subject to the same limitations. As soon as we can use rsync, we can build the website and deploy it from the same job. We don't need to use artifacts at all.
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 explaining. It sounds like we can initially ship this separate workflow and merge it with the build workflow once playground.wordpress.net is totally moved to the new host.
Though it is not required, do you see any value in just saving a build artifact to keep a record of what was actually deployed?
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.
Though it is not required, do you see any value in just saving a build artifact to keep a record of what was actually deployed?
@brandonpayton Not at the moment:
- I never had to go back to review previous artifacts
- The current deployment workflow keeps a copy of the last website version for quick rollbacks if needed
- Rebuilding a historical commit is fast
I'm trying to think of a scenario where this would be useful and nothing comes up. Am I missing anything?
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.
@adamziel, I don't think you're missing anything. I asked because I like reproducibility but didn't really have an opinion on whether it mattered here. :) Thanks, we can just plan to drop the artifact in the future.
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 and the deployment seems to have succeeded
What is this PR doing?
This PR adds a workflow for deploying the playground.wordpress.net website to WP Cloud
Related to #1197
What problem is it solving?
Sometimes the website has availability issues due to its current shared hosting environment. Switching to WP Cloud is intended to address those issues.
Testing Instructions
NOTE: There are some outstanding issues with rate-limiting that break certain aspects of WordPress Playground on the test site, but Playground should still load the WP home page. The rate-limiting issues will be addressed separately, and possibly with the host itself rather than in a follow-up PR.