From ba6f445b8cd872a9afffd45019e5e29d18223585 Mon Sep 17 00:00:00 2001 From: Fred Zhang Date: Tue, 21 May 2024 08:10:47 -0700 Subject: [PATCH 1/6] force sql:migrate --- src/dataconnect/schemaMigration.ts | 46 +++++++++++++++++++----------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/src/dataconnect/schemaMigration.ts b/src/dataconnect/schemaMigration.ts index cf49c33a092..983f6ecef64 100644 --- a/src/dataconnect/schemaMigration.ts +++ b/src/dataconnect/schemaMigration.ts @@ -79,15 +79,11 @@ export async function migrateSchema(args: { const shouldDeleteInvalidConnectors = await promptForInvalidConnectorError( options, + serviceName, invalidConnectors, validateOnly, ); - if (!shouldDeleteInvalidConnectors && invalidConnectors.length) { - const cmd = suggestedCommand(serviceName, invalidConnectors); - throw new FirebaseError( - `Command aborted. Try deploying those connectors first with ${clc.bold(cmd)}`, - ); - } + const migrationMode = incompatible ? await promptForSchemaMigration( options, @@ -112,7 +108,7 @@ export async function migrateSchema(args: { }); } - if (invalidConnectors.length) { + if (shouldDeleteInvalidConnectors) { await deleteInvalidConnectors(invalidConnectors); } // Then, try to upsert schema again. If there still is an error, just throw it now @@ -212,7 +208,7 @@ async function promptForSchemaMigration( ): Promise<"none" | "safe" | "all"> { displaySchemaChanges(err); if (!options.nonInteractive) { - // Always prompt in interactive mode. Desturctive migrations are too potentially dangerous to not prompt for with --force + // Always prompt in interactive mode. Destructive migrations are too potentially dangerous to not prompt for with --force const choices = err.destructive ? [ { name: "Execute all changes (including destructive changes)", value: "all" }, @@ -250,6 +246,7 @@ async function promptForSchemaMigration( async function promptForInvalidConnectorError( options: Options, + serviceName: string, invalidConnectors: string[], validateOnly: boolean, ): Promise { @@ -258,18 +255,33 @@ async function promptForInvalidConnectorError( } displayInvalidConnectors(invalidConnectors); if (validateOnly) { - return false; - } else if ( - options.force || - (!options.nonInteractive && - (await confirm({ - ...options, - message: "Would you like to delete and recreate these connectors?", - }))) + if (options.force) { + // `firebase dataconnect:sql:migrate --force` ignores invalid connectors. + return false; + } + // `firebase dataconnect:sql:migrate` aborts if there are invalid connectors. + throw new FirebaseError( + `Command aborted. If you'd like to migrate it anyway, you may override with --force.`, + ); + } + if (options.force) { + // `firebase deploy --force` will delete invalid connectors without prompting. + return true; + } + // `firebase deploy` prompts in case of invalid connectors. + if ( + !options.nonInteractive && + (await confirm({ + ...options, + message: "Would you like to delete and recreate these connectors?", + })) ) { return true; } - return false; + const cmd = suggestedCommand(serviceName, invalidConnectors); + throw new FirebaseError( + `Command aborted. Try deploying those connectors first with ${clc.bold(cmd)}`, + ); } async function deleteInvalidConnectors(invalidConnectors: string[]): Promise { From ea68811e6ded75f9fe72cc0e76139f9ebcc69835 Mon Sep 17 00:00:00 2001 From: Fred Zhang Date: Tue, 21 May 2024 08:18:23 -0700 Subject: [PATCH 2/6] switch back to pg.Pool --- src/gcp/cloudsql/connect.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/gcp/cloudsql/connect.ts b/src/gcp/cloudsql/connect.ts index 5bb78a1b7bc..2397649568b 100644 --- a/src/gcp/cloudsql/connect.ts +++ b/src/gcp/cloudsql/connect.ts @@ -32,7 +32,7 @@ export async function execute( ); } let connector: Connector; - let client: pg.Client; + let pool: pg.Pool; switch (user.type) { case "CLOUD_IAM_USER": { connector = new Connector({ @@ -43,7 +43,7 @@ export async function execute( ipType: IpAddressTypes.PUBLIC, authType: AuthTypes.IAM, }); - client = new pg.Client({ + pool = new pg.Pool({ ...clientOpts, user: opts.username, database: opts.databaseId, @@ -60,7 +60,8 @@ export async function execute( ipType: IpAddressTypes.PUBLIC, authType: AuthTypes.IAM, }); - client = new pg.Client({ + console.log("CLOUD_IAM_SERVICE_ACCOUNT", user, "clientOpts", clientOpts); + pool = new pg.Pool({ ...clientOpts, user: opts.username, database: opts.databaseId, @@ -79,7 +80,7 @@ export async function execute( instanceConnectionName: connectionName, ipType: IpAddressTypes.PUBLIC, }); - client = new pg.Client({ + pool = new pg.Pool({ ...clientOpts, user: opts.username, password: opts.password, @@ -89,6 +90,7 @@ export async function execute( } } + const client = await pool.connect(); logFn(`Logged in as ${opts.username}`); for (const s of sqlStatements) { logFn(`Executing: '${s}'`); @@ -99,7 +101,8 @@ export async function execute( } } - await client.end(); + // This hangs somehow. + // await pool.end(); connector.close(); } From 2374c9ee503b568d79d1053ac88b2e5fb1853491 Mon Sep 17 00:00:00 2001 From: Fred Zhang Date: Tue, 21 May 2024 08:19:09 -0700 Subject: [PATCH 3/6] m --- src/gcp/cloudsql/connect.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/gcp/cloudsql/connect.ts b/src/gcp/cloudsql/connect.ts index 2397649568b..1e3ccd8a716 100644 --- a/src/gcp/cloudsql/connect.ts +++ b/src/gcp/cloudsql/connect.ts @@ -60,7 +60,6 @@ export async function execute( ipType: IpAddressTypes.PUBLIC, authType: AuthTypes.IAM, }); - console.log("CLOUD_IAM_SERVICE_ACCOUNT", user, "clientOpts", clientOpts); pool = new pg.Pool({ ...clientOpts, user: opts.username, From 391686be25afdef2b6508265849caeead3df18f0 Mon Sep 17 00:00:00 2001 From: Fred Zhang Date: Tue, 21 May 2024 08:23:31 -0700 Subject: [PATCH 4/6] m --- src/gcp/cloudsql/connect.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/gcp/cloudsql/connect.ts b/src/gcp/cloudsql/connect.ts index 1e3ccd8a716..ad9a14834fc 100644 --- a/src/gcp/cloudsql/connect.ts +++ b/src/gcp/cloudsql/connect.ts @@ -89,19 +89,19 @@ export async function execute( } } - const client = await pool.connect(); + const conn = await pool.connect(); logFn(`Logged in as ${opts.username}`); for (const s of sqlStatements) { logFn(`Executing: '${s}'`); try { - await client.query(s); + await conn.query(s); } catch (err) { throw new FirebaseError(`Error executing ${err}`); } } - // This hangs somehow. - // await pool.end(); + conn.release(); + await pool.end(); connector.close(); } From c02693963a586bd331f671545c233e4f4be2cce2 Mon Sep 17 00:00:00 2001 From: Fred Zhang Date: Tue, 21 May 2024 08:36:46 -0700 Subject: [PATCH 5/6] wording --- src/dataconnect/schemaMigration.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dataconnect/schemaMigration.ts b/src/dataconnect/schemaMigration.ts index 983f6ecef64..73b1b6a9394 100644 --- a/src/dataconnect/schemaMigration.ts +++ b/src/dataconnect/schemaMigration.ts @@ -273,7 +273,7 @@ async function promptForInvalidConnectorError( !options.nonInteractive && (await confirm({ ...options, - message: "Would you like to delete and recreate these connectors?", + message: `Would you like to delete and recreate these connectors? This will cause ${clc.red(`downtime.`)}.`, })) ) { return true; @@ -296,7 +296,7 @@ function displayInvalidConnectors(invalidConnectors: string[]) { ); logLabeledWarning( "dataconnect", - `This is a ${clc.red("breaking")} change and will cause a brief downtime.`, + `This is a ${clc.red("breaking")} change and may break existing apps.`, ); } From 6e523de462059c9a492f5735eea8bdee359c703d Mon Sep 17 00:00:00 2001 From: Fred Zhang Date: Tue, 21 May 2024 08:40:56 -0700 Subject: [PATCH 6/6] wording --- src/dataconnect/schemaMigration.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/dataconnect/schemaMigration.ts b/src/dataconnect/schemaMigration.ts index 73b1b6a9394..588e3b4c84f 100644 --- a/src/dataconnect/schemaMigration.ts +++ b/src/dataconnect/schemaMigration.ts @@ -111,8 +111,10 @@ export async function migrateSchema(args: { if (shouldDeleteInvalidConnectors) { await deleteInvalidConnectors(invalidConnectors); } - // Then, try to upsert schema again. If there still is an error, just throw it now - await upsertSchema(schema, validateOnly); + if (!validateOnly) { + // Then, try to upsert schema again. If there still is an error, just throw it now + await upsertSchema(schema, validateOnly); + } return diffs; } return [];