[Tooling]: Added caching for Playwright (tests), and backend docker container when checking auto-generated API code - #1660
Conversation
There was a problem hiding this comment.
Pull request overview
This PR speeds up the frontend CI job by caching two large/expensive dependencies: Playwright browser binaries used by component tests, and Docker build layers for the backend image used during the “generated API code sync” check.
Changes:
- Add an
actions/cachestep to persist/restore Playwright browser binaries keyed byfrontend/package-lock.json, and conditionally skip the browser download on cache hits. - Switch the backend image build in the API sync check from
docker buildtodocker buildx buildwith GitHub Actions cache backend (type=gha) to reuse Docker layers across runs.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
anders-kiaer
left a comment
There was a problem hiding this comment.
Looks good - would be nice to improve frontend CI speed 👍
Some questions/comments below, otherwise LGTM.
| --cache-from type=local,src=/tmp/buildx-cache \ | ||
| --cache-to type=local,dest=/tmp/buildx-cache-new,mode=max \ |
There was a problem hiding this comment.
I see we tried the buildx cache backend built for GitHub actions in an earlier commit (type=gha). What was the reason we wanted to change to local? Should we add a comment here stating the reason?
| docker build -f ./backend_py/primary/Dockerfile -t backend:latest . | ||
| CONTAINER_ID=$(docker run --detach -p 5000:5000 --env UVICORN_PORT=5000 --env WEBVIZ_SKIP_LIFESPAN_GENERATE_API_ONLY=true --env WEBVIZ_CLIENT_SECRET=0 --env WEBVIZ_SMDA_SUBSCRIPTION_KEY=0 --env WEBVIZ_VDS_HOST_ADDRESS=0 --env WEBVIZ_ENTERPRISE_SUBSCRIPTION_KEY=0 backend:latest) | ||
| sleep 10 # Ensure the backend server is up and running exposing /openapi.json | ||
| set -euo pipefail |
There was a problem hiding this comment.
Is this needed? -e seems default on GitHub actions, and we do not use pipes in this job?
There was a problem hiding this comment.
You are right that -e is enabled by default and that we don't have any pipes. However, adding -u adds immediate value by adding fail-fast behaviour. -o pipefail is an insurance for future changes to the step as it discovers any hidden failures (e.g. some_command | grep ... where grep is successful but some_command fails). So yes, theoretically, we could only add -u but set -euo pipefail is a good default and immediately shows what is set (without having to know about and/or rely on GitHub).
| # Single-quoted so $CONTAINER_ID is expanded at exit time, not now. | ||
| trap 'docker stop "$CONTAINER_ID"' EXIT | ||
|
|
||
| sleep 10 |
There was a problem hiding this comment.
Replacing this by something more explicit which also continues immediately when server is up?
| sleep 10 | |
| # Waiting on OpenAPI server up and running: | |
| timeout 10 bash -c 'until curl -sf http://localhost:5000/openapi.json > /dev/null 2>&1; do sleep 1; done' |
| # Single-quoted so $CONTAINER_ID is expanded at exit time, not now. | ||
| trap 'docker stop "$CONTAINER_ID"' EXIT |
There was a problem hiding this comment.
Good and more robust replacement of docker stop $CONTAINER_ID. We could also consider removing this completely as GitHub actions jobs are independent it should/will stop automatically when the job ends.
Cache CI dependencies to speed up the frontend job
Summary
frontend/package-lock.json. On a cache hit, only OS-level system dependencies are installed (install-deps); the ~300 MB browser download is skipped.type=gha,mode=max). Unchanged layers (e.g. the Poetry dependency install) are reused across runs; only layers whose inputs have changed are rebuilt.Cache invalidation
Playwright — the cache key includes a hash of
package-lock.json, so it is automatically busted whenever the Playwright version is bumped.Docker — invalidation is content-addressed at the layer level by Docker itself. The
COPY ./backend_py/primaryinstruction in the Dockerfile means any change to backend source files (routes, schemas, response models) will bust that layer and all subsequent ones, ensuring the API sync check always runs against the correct backend image.Test plan