Repository navigation
[No QA] Stop syncing the test-staging route on Worker deploys - #104002
Conversation
wrangler looks up the zone before syncing routes, which the CI deploy token cannot do, so the deploy job failed after the Worker was already live. The route stays in place in Cloudflare; wrangler only replaces routes when some are configured.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Is expanding this token's permissions a viable option? Just asking because generally my preference is to have as much config in code as possible, as opposed to config in dashboards |
| // access, and the CI deploy token can only edit this Worker. | ||
| "test-staging": { | ||
| "name": "test-staging-new-expensify-assets", | ||
| "routes": [{"pattern": "test-staging.new.expensify.com/*", "zone_name": "expensify.com"}], |
There was a problem hiding this comment.
Probably a good idea to not keep this here too as a bad edit to the route here can bring the site down
|
@roryabraham yeah it is but I thought the counter point was a good reason to not have this here so that someone can't accidentally change something in the route and bring our site down. The route should never change unless we are migrating (sub)domains |
|
🚧 justinpersaud has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/justinpersaud in version: 9.5.10-0 🚀
|
|
🚀 Deployed to staging by https://github.com/justinpersaud in version: 9.5.12-0 🚀
|
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.5.12-1 🚀
Bundle Size Analysis (Sentry): |
4 participants
Explanation of Change
The first automatic test-staging deploy (run) uploaded and deployed the Worker, so 9.5.9-0 is live on
test-staging.new.expensify.com, but then failed withCould not find zone for expensify.com. When routes are configured,wrangler deploylooks up their zone to sync them, andCLOUDFLARE_WORKERS_DEPLOY_TOKENcan only edit the test-staging Worker, not read zones.This removes the route from
env.test-staginginwrangler.jsonc. The route already exists in Cloudflare and keeps working: wrangler 4.144.0 only replaces a Worker's routes when the config lists some, so a deploy with none leaves it untouched. The token stays limited to the one Worker. The README and a config comment note that the route is managed in Cloudflare.Fixed Issues
For https://github.com/Expensify/Expensify/issues/688126
PROPOSAL:
Tests
Offline tests
N/A. Deploy configuration only.
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
No QA. Only changes how the test-staging Worker is deployed; staging and production are unaffected.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari