Repository navigation
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces support for deploying Cloud Run services directly via the Firebase CLI under the 'direct_cloud_run' experiment, adding the 'run' deploy target, integration with 'firebase init', and comprehensive tests. The review feedback suggests several key improvements: handling expected user-facing errors by importing and throwing 'FirebaseError', avoiding non-null assertions on the main container template to prevent runtime errors, ensuring temporary local archives are cleaned up in a 'finally' block to avoid disk leaks, parallelizing service preparation using 'Promise.all' for better performance, and adding defensive checks for potentially undefined configuration properties.
Contributor
Author
|
/gemini review |
falahat
added this pull request to stack #11209
September 30, 2026 04:19
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces support for deploying Cloud Run services directly from local source using the Firebase CLI. It adds the new run deploy target, implements its prepare, deploy, and release phases, integrates it into the initialization flow, and adds comprehensive unit and end-to-end tests. The review feedback highlights several key improvements: adhering to strict null checks by avoiding the non-null assertion operator on the main container, cleaning up temporary local archive files after upload to prevent disk space leaks, verifying the existence of the deployed service URI before logging, and using npx mocha in the test script for better environment robustness.
falahat
force-pushed
the
bapi_deploy_run
branch
from
September 30, 2026 21:37
3ba264a to
1086ebe
Compare
falahat
removed this pull request from stack #11209
September 30, 2026 21:42
falahat
added this pull request to stack #11226
October 2, 2026 16:41
A base image turns on automatic base image updates, which are off by default. So init no longer suggests nodejs22. An existing service still defaults to its own base image.
Spell out what init changes on an existing service (rootDir and region) instead of merging three spreads. upsertRunConfig now adds the default ignore list itself, so actuate no longer passes it. Behavior is unchanged; added a test for the ignore fallback on existing services.
Spell out what init changes on an existing service (rootDir and region) instead of merging three spreads. upsertRunConfig now adds the default ignore list itself, so actuate no longer passes it. Behavior is unchanged; added a test for the ignore fallback on existing services.
Re-running init on a service that's already in firebase.json filled in the default ignore list when the entry didn't have one. Init doesn't ask about ignore, so it shouldn't change it: an existing entry now only gets the rootDir and region the user just chose. New services still get the default ignore list.
…ce ID and region Pressing Enter now keeps the root directory saved in firebase.json instead of resetting it to /. Cloud Run service IDs are only unique within a region, so entries now match on both; the same ID in another region gets its own entry.
…gions Init deploys with --only run:<id>, which matches that ID in every region. Init now also passes the service's region to deploy, which skips services in other regions. firebase deploy and --only run:<id> are unchanged.
…actuate Init already writes firebase.json after every feature is set up, so the write in actuate only printed "Wrote configuration info to firebase.json" a second time. Leaving the write to init also means a failed init leaves firebase.json untouched.
# Conflicts: # src/init/features/run.spec.ts # src/init/features/run.ts
scripts/run-deploy-tests ran the real CLI against a real project. Nothing ran it automatically (npm test only covers src/, and CI doesn't list it), and each run left Cloud Build images behind in Artifact Registry. The unit tests cover the logic, and live testing is done by hand.
# Conflicts: # src/deploy/run/util.spec.ts # src/deploy/run/util.ts # src/init/features/run.spec.ts # src/init/features/run.ts
falahat
commented
Oct 8, 2026
Service IDs are only unique within a region, so firebase.json can list the same ID in more than one region. - run:<serviceId> still deploys that ID in every region it's listed in, and now logs which services it matched when there's more than one. - run:<serviceId>:<region> deploys exactly one service. Init deploys the service it set up this way, which replaces the region it used to pass in the deploy context. - Per-service messages (uploading, building, deployed) include the region. - firebase.json can't list the same service ID and region twice.
- deploy.ts reads as four steps: upload the source, build it, prepare the revision, then create the service or roll out a new revision. The revision rules are a pure function, revisionTemplate, with one small test per rule. - Helpers that only deploy.ts uses (deployRevision, toAppHostingConfig, the rollout timeout) moved there from util.ts. util.ts now only picks services from firebase.json and checks whether a service exists. - Stop deleting the local source zip. It's a tmp file, which firebase deploy deletes when it exits; App Hosting relies on the same thing. - Check every service's region once, up front, and stop --only filtering at the first service that isn't in firebase.json. - Tests use realistic names, check only the fields they're about, and drop duplicate coverage. The release test has its own file. - The help text no longer mentions local builds or run:services:update, which later PRs add. - Clearer docstrings and tests for init run's actuate and upsertRunConfig.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds
firebase deploy --only run, behind thedirectcloudrunexperiment. For each Cloud Run service in firebase.json, it builds the local source on Cloud Build, then creates the service or rolls out a new revision.firebase init runnow deploys the service it sets up.prepare.ts: picks the services, reads them from Cloud Run, and keeps their base image.deploy.ts: uploads the source, builds it, then creates the service or rolls out a new revision.revisionTemplatekeeps the live revision's settings.release.ts: logs each service's URL.util.ts: handles--only run,run:<serviceId>, andrun:<serviceId>:<region>.run:services:update(#11206), local builds (#11207), and SDK auto-init (#11208) come in follow-up PRs.Scenarios Tested
-m, and used--onlywith the same service ID in two regions.Sample Commands