-
Notifications
You must be signed in to change notification settings - Fork 929
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 --dry-run to deploy #7574
Add --dry-run to deploy #7574
Conversation
src/deploy/functions/prepare.ts
Outdated
@@ -276,9 +277,12 @@ export async function prepare( | |||
// ===Phase 7. Finalize preparation by "fixing" all extraneous environment issues like IAM policies. | |||
// We limit the scope endpoints being deployed. | |||
await backend.checkAvailability(context, matchingBackend); | |||
await ensureServiceAgentRoles(projectId, projectNumber, matchingBackend, haveBackend); | |||
// TODO: CheckServiceAgentRoles when dryRun = true |
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 assume you will do this TODO before you submit this PR?
src/deploy/functions/prepare.ts
Outdated
await validate.secretsAreValid(projectId, matchingBackend); | ||
await ensure.secretAccess(projectId, matchingBackend, haveBackend); | ||
// TODO: CheckSecretAccess when dryRun = true |
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.
Also here
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.
Sorry for the sloppiness here, and ty for the detailed reviews!
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7574 +/- ##
==========================================
- Coverage 53.06% 52.79% -0.28%
==========================================
Files 391 394 +3
Lines 27003 27525 +522
Branches 5571 5687 +116
==========================================
+ Hits 14330 14532 +202
- Misses 11393 11703 +310
- Partials 1280 1290 +10 ☔ View full report in Codecov by Sentry. |
@@ -55,6 +58,36 @@ export default async function (context: any, options: Options): Promise<void> { | |||
filters, | |||
}; | |||
utils.logLabeledBullet("dataconnect", `Successfully prepared schema and connectors`); | |||
if (options.dryRun) { |
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.
From chatting offline -- we should invoke diffSchema
here to surface migration diffs in dryRun
Description
Adds a new
--dry-run
flag tofirebase deploy
.firebase deploy --dry-run
will run any validations or build steps, and then report information about what change would have been made, without making any changes to your project.Under the hood, this means that we only run the prepare step, and skip release/deploy. I reviewed
prepare.ts
for each product to ensure they don't make any production changes, and moved some validation from later stages to prepare where it belongs.