address feedback

Author: blidd-google

CHANGELOG.md

@@ -1 +1 @@
-- Add new 2nd gen Firestore auth context triggers. (#1519)
+- Add new 2nd gen Firestore triggered functions with auth context. (#1519)

src/deploy/functions/prompts.ts

@@ -76,7 +76,7 @@ export async function promptForFailurePolicies(
  */
 export async function promptForFunctionDeletion(
   functionsToDelete: (backend.TargetIds & { platform: backend.FunctionsPlatform })[],
-  options: Options,
+  options: Options
 ): Promise<boolean> {
   let shouldDeleteFns = true;
   if (functionsToDelete.length === 0 || options.force) {
@@ -127,7 +127,7 @@ export async function promptForFunctionDeletion(
  */
 export async function promptForUnsafeMigration(
   fnsToUpdate: EndpointUpdate[],
-  options: Options,
+  options: Options
 ): Promise<EndpointUpdate[]> {
   const unsafeUpdates = fnsToUpdate.filter((eu) => eu.unsafe);
 
@@ -142,11 +142,11 @@ export async function promptForUnsafeMigration(
         .map((eu) => eu.endpoint)
         .sort(backend.compareFunctions)
         .map(getFunctionLabel)
-        .join(", "),
+        .join(", ")
     ) +
     ". " +
     "While automatic migration is allowed for these functions, updating the underlying event type may result in data loss. " +
-    "To avoid this, consider the best practices outlined in the migration guide: [TODO: link]";
+    "To avoid this, consider the best practices outlined in the migration guide: https://firebase.google.com/docs/functions/manage-functions?gen=2nd#modify-trigger";
 
   utils.logLabeledWarning("functions", warnMessage);
 
@@ -155,7 +155,7 @@ export async function promptForUnsafeMigration(
   if (options.nonInteractive) {
     utils.logLabeledWarning(
       "functions",
-      "Aborting updates for functions that may be unsafe to update. To update these functions anyway, deploy again in interactive mode or use the --force option.",
+      "Skipping updates for functions that may be unsafe to update. To update these functions anyway, deploy again in interactive mode or use the --force option."
     );
     return safeUpdates;
   }
@@ -166,7 +166,7 @@ export async function promptForUnsafeMigration(
       name: "confirm",
       default: false,
       message: `[${getFunctionLabel(
-        eu.endpoint,
+        eu.endpoint
       )}] Would you like to proceed with the unsafe migration?`,
     });
     if (shouldUpdate) {

src/deploy/functions/release/fabricator.ts

@@ -102,7 +102,7 @@ export class Fabricator {
     if (errs.length) {
       logger.debug(
         "Fabricator.applyRegionalChanges returned an unhandled exception. This should never happen",
-        JSON.stringify(errs, null, 2),
+        JSON.stringify(errs, null, 2)
       );
     }
 
@@ -115,7 +115,7 @@ export class Fabricator {
     const handle = async (
       op: reporter.OperationType,
       endpoint: backend.Endpoint,
-      fn: () => Promise<void>,
+      fn: () => Promise<void>
     ): Promise<void> => {
       const timer = new Timer();
       const result: Partial<reporter.DeployResult> = { endpoint };
@@ -135,7 +135,7 @@ export class Fabricator {
     for (const endpoint of changes.endpointsToCreate) {
       this.logOpStart("creating", endpoint);
       upserts.push(
-        handle("create", endpoint, () => this.createEndpoint(endpoint, scraperV1, scraperV2)),
+        handle("create", endpoint, () => this.createEndpoint(endpoint, scraperV1, scraperV2))
       );
     }
     for (const endpoint of changes.endpointsToSkip) {
@@ -144,7 +144,7 @@ export class Fabricator {
     for (const update of changes.endpointsToUpdate) {
       this.logOpStart("updating", update.endpoint);
       upserts.push(
-        handle("update", update.endpoint, () => this.updateEndpoint(update, scraperV1, scraperV2)),
+        handle("update", update.endpoint, () => this.updateEndpoint(update, scraperV1, scraperV2))
       );
     }
     await utils.allSettled(upserts);
@@ -175,7 +175,7 @@ export class Fabricator {
   async createEndpoint(
     endpoint: backend.Endpoint,
     scraperV1: SourceTokenScraper,
-    scraperV2: SourceTokenScraper,
+    scraperV2: SourceTokenScraper
   ): Promise<void> {
     endpoint.labels = { ...endpoint.labels, ...deploymentTool.labels() };
     if (endpoint.platform === "gcfv1") {
@@ -192,7 +192,7 @@ export class Fabricator {
   async updateEndpoint(
     update: planner.EndpointUpdate,
     scraperV1: SourceTokenScraper,
-    scraperV2: SourceTokenScraper,
+    scraperV2: SourceTokenScraper
   ): Promise<void> {
     update.endpoint.labels = { ...update.endpoint.labels, ...deploymentTool.labels() };
     if (update.deleteAndRecreate) {
@@ -398,7 +398,7 @@ export class Fabricator {
       logger.debug("Result function unexpectedly didn't have a service name.");
       utils.logLabeledWarning(
         "functions",
-        "Updated function is not associated with a service. This deployment is in an unexpected state - please re-deploy your functions.",
+        "Updated function is not associated with a service. This deployment is in an unexpected state - please re-deploy your functions."
       );
       return;
     }
@@ -511,7 +511,7 @@ export class Fabricator {
             onPoll: scraper.poller,
           });
         },
-        { retryCodes: [...DEFAULT_RETRY_CODES, CLOUD_RUN_RESOURCE_EXHAUSTED_CODE] },
+        { retryCodes: [...DEFAULT_RETRY_CODES, CLOUD_RUN_RESOURCE_EXHAUSTED_CODE] }
       )
       .catch((err: any) => {
         scraper.abort();
@@ -526,7 +526,7 @@ export class Fabricator {
       logger.debug("Result function unexpectedly didn't have a service name.");
       utils.logLabeledWarning(
         "functions",
-        "Updated function is not associated with a service. This deployment is in an unexpected state - please re-deploy your functions.",
+        "Updated function is not associated with a service. This deployment is in an unexpected state - please re-deploy your functions."
       );
       return;
     }
@@ -581,7 +581,7 @@ export class Fabricator {
           };
           await poller.pollOperation<void>(pollerOptions);
         },
-        { retryCodes: [...DEFAULT_RETRY_CODES, CLOUD_RUN_RESOURCE_EXHAUSTED_CODE] },
+        { retryCodes: [...DEFAULT_RETRY_CODES, CLOUD_RUN_RESOURCE_EXHAUSTED_CODE] }
       )
       .catch(rethrowAs(endpoint, "delete"));
   }
@@ -686,7 +686,7 @@ export class Fabricator {
   }
 
   async registerBlockingTrigger(
-    endpoint: backend.Endpoint & backend.BlockingTriggered,
+    endpoint: backend.Endpoint & backend.BlockingTriggered
   ): Promise<void> {
     await this.executor
       .run(() => services.serviceForEndpoint(endpoint).registerTrigger(endpoint))
@@ -723,7 +723,7 @@ export class Fabricator {
   }
 
   async unregisterBlockingTrigger(
-    endpoint: backend.Endpoint & backend.BlockingTriggered,
+    endpoint: backend.Endpoint & backend.BlockingTriggered
   ): Promise<void> {
     await this.executor
       .run(() => services.serviceForEndpoint(endpoint).unregisterTrigger(endpoint))
@@ -736,7 +736,7 @@ export class Fabricator {
     const label = helper.getFunctionLabel(endpoint);
     utils.logLabeledBullet(
       "functions",
-      `${op} ${runtime} (${platform}) function ${clc.bold(label)}...`,
+      `${op} ${runtime} (${platform}) function ${clc.bold(label)}...`
     );
   }
 
@@ -764,7 +764,7 @@ export class Fabricator {
     const functionNames = endpoints.map((endpoint) => endpoint.id).join(",");
     return `${clc.bold(clc.magenta(`functions:`))} You can re-deploy skipped functions with:
               ${clc.bold(`firebase deploy --only functions:${functionNames}`)} or ${clc.bold(
-                `FUNCTIONS_DEPLOY_UNCHANGED=true firebase deploy`,
-              )}`;
+      `FUNCTIONS_DEPLOY_UNCHANGED=true firebase deploy`
+    )}`;
   }
 }

src/deploy/functions/release/index.ts

@@ -66,7 +66,7 @@ export async function release(
   }
   for (const eu of fnsToUpdateSafe) {
     const e = eu.endpoint;
-    const key = `${e.codebase}-${e.region}-${e.availableMemoryMb || "default"}`;
+    const key = `${e.codebase || ""}-${e.region}-${e.availableMemoryMb || "default"}`;
     plan[key].endpointsToUpdate.push(eu);
   }