SW-8251 Port terraware-web CI workflow to Buildkite#5545
SW-8251 Port terraware-web CI workflow to Buildkite#5545constanzauanini wants to merge 6 commits intomainfrom
Conversation
constanzauanini
force-pushed
the
constanza/SW-8251
branch
from
25ac989 to
bbb4a43
Compare
constanzauanini
force-pushed
the
constanza/SW-8251
branch
from
bbb4a43 to
f260824
Compare
sgrimm
left a comment
sgrimm
left a comment
There was a problem hiding this comment.
Seems pretty close. My original terraware-server PR got a few things wrong that I've since fixed, so some of my comments are about those things.
.buildkite/scripts/deploy-vercel.sh
Outdated
| .buildkite/scripts/install-deps.sh --node --tools | ||
|
|
||
| echo "--- :vercel: Install Vercel CLI" | ||
| npm install -g vercel |
There was a problem hiding this comment.
Can we remove the -g here so we don't install this on the host, just in the repo's node_modules?
| buildkite-agent annotate \ | ||
| --style info \ | ||
| --context vercel-preview \ | ||
| "Vercel preview: [${preview_url}](${preview_url})" |
.buildkite/scripts/install-deps.sh
Outdated
| case "$arg" in | ||
| --node) install_node ;; | ||
| --tools) install_jq && install_yq && install_rsync ;; | ||
| --playwright-deps) install_playwright_system_deps ;; |
There was a problem hiding this comment.
Maybe rename this to --playwright so it's more consistent with the other arguments.
.buildkite/pipeline.yml
Outdated
| - *shallow-clone | ||
| - cache#v1.10.0: | ||
| <<: *cache-node-modules-args | ||
| manifest: 'yarn.lock' |
There was a problem hiding this comment.
Probably makes sense to move this up to cache-node-modules-args.
.buildkite/pipeline.yml
Outdated
| save: | ||
| - file | ||
| - branch | ||
| force: true |
There was a problem hiding this comment.
I think you shouldn't need force here or on the item below this. I had it in my original terraware-server Buildkite PR but later I removed it because it was causing unnecessary cache updates.
.buildkite/pipeline.yml
Outdated
| path: '.cache/ms-playwright' | ||
| manifest: 'yarn.lock' |
There was a problem hiding this comment.
Could create a cache-playwright-args anchor and move these there so they don't need to be repeated below.
There was a problem hiding this comment.
I will remove this, it is only needed for e2e tests
.buildkite/pipeline.yml
Outdated
| - cache#v1.10.0: &promote-cache | ||
| <<: *cache-node-modules-args | ||
| save: 'pipeline' | ||
| force: true |
There was a problem hiding this comment.
No need for force here if you add the manifest to cache-node-modules-args.
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| (numBatches === 1 ? strings.BATCHES_SINGULAR : strings.BATCHES_PLURAL) as any, | ||
| totalWithdrawn, | ||
| totalWithdrawn === 1 ? strings.SEEDLINGS_SINGULAR : (strings.SEEDLINGS_PLURAL as any) | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| (totalWithdrawn === 1 ? strings.SEEDLINGS_SINGULAR : strings.SEEDLINGS_PLURAL) as any |
There was a problem hiding this comment.
These seem unrelated to the Buildkite build.
There was a problem hiding this comment.
This made the linter step fail, that's why I added, not sure how it was not failing before
constanzauanini
force-pushed
the
constanza/SW-8251
branch
from
80ac6bc to
7e87614
Compare
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| - label: ':amazon-ecs: Deploy to ECS' | ||
| key: 'deploy' | ||
| depends_on: 'promote-caches' | ||
| if: "build.branch == 'main' || build.tag =~ /^v[0-9]+\\.[0-9]+/" | ||
| plugins: | ||
| - *shallow-clone | ||
| command: .buildkite/scripts/deploy-ecs.sh |
There was a problem hiding this comment.
Critical dependency chain bug: The deploy step depends on promote-caches, but promote-caches only runs on the main branch (line 95). However, deploy is configured to run on both main branch AND version tags (line 109). When deploying from a version tag like v1.2.3, the promote-caches step will be skipped because it's not on the main branch, causing the deploy step to be blocked waiting for a dependency that will never execute.
Fix:
- label: ':amazon-ecs: Deploy to ECS'
key: 'deploy'
depends_on: 'docker' # Change from 'promote-caches' to 'docker'
if: "build.branch == 'main' || build.tag =~ /^v[0-9]+\\.[0-9]+/"
plugins:
- *shallow-clone
command: .buildkite/scripts/deploy-ecs.shOr add conditional dependency handling if Buildkite supports it.
| - label: ':amazon-ecs: Deploy to ECS' | |
| key: 'deploy' | |
| depends_on: 'promote-caches' | |
| if: "build.branch == 'main' || build.tag =~ /^v[0-9]+\\.[0-9]+/" | |
| plugins: | |
| - *shallow-clone | |
| command: .buildkite/scripts/deploy-ecs.sh | |
| - label: ':amazon-ecs: Deploy to ECS' | |
| key: 'deploy' | |
| depends_on: 'docker' | |
| if: "build.branch == 'main' || build.tag =~ /^v[0-9]+\\.[0-9]+/" | |
| plugins: | |
| - *shallow-clone | |
| command: .buildkite/scripts/deploy-ecs.sh | |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
2 participants
I did this with Claude and based on @sgrimm server pr