tweaks

src/dataconnect/client.ts

@@ -80,8 +80,9 @@ export async function deleteService(
   locationId: string,
   serviceId: string,
 ): Promise<types.Service> {
+  // NOTE(fredzqm): Don't force delete yet. Backend would leave orphaned resources.
   const op = await dataconnectClient().delete<types.Service>(
-    `projects/${projectId}/locations/${locationId}/services/${serviceId}?force=true`,
+    `projects/${projectId}/locations/${locationId}/services/${serviceId}`,
   );
   const pollRes = await operationPoller.pollOperation<types.Service>({
     apiOrigin: dataconnectOrigin(),

src/dataconnect/provisionCloudSql.ts

@@ -77,21 +77,11 @@ export async function provisionCloudSql(args: {
   try {
     await cloudSqlAdminClient.getDatabase(projectId, instanceId, databaseId);
     silent || utils.logLabeledBullet("dataconnect", `Found existing database ${databaseId}.`);
-  } catch (err: any) {
-    if (err.status === 404) {
-      // Create the database if not found.
-      silent ||
-        utils.logLabeledBullet(
-          "dataconnect",
-          `Database ${databaseId} not found, creating it now...`,
-        );
-      await cloudSqlAdminClient.createDatabase(projectId, instanceId, databaseId);
-      silent || utils.logLabeledBullet("dataconnect", `Database ${databaseId} created.`);
-    } else {
-      // Skip it if the database is not accessible.
-      // Possible that the CSQL instance is in the middle of something.
-      silent || utils.logLabeledWarning("dataconnect", `Database ${databaseId} is not accessible.`);
-    }
+  } catch (err) {
+    silent ||
+      utils.logLabeledBullet("dataconnect", `Database ${databaseId} not found, creating it now...`);
+    await cloudSqlAdminClient.createDatabase(projectId, instanceId, databaseId);
+    silent || utils.logLabeledBullet("dataconnect", `Database ${databaseId} created.`);
   }
   if (enableGoogleMlIntegration) {
     await grantRolesToCloudSqlServiceAccount(projectId, instanceId, [GOOGLE_ML_INTEGRATION_ROLE]);

src/dataconnect/schemaMigration.ts

@@ -12,13 +12,24 @@ import { FirebaseError } from "../error";
 import { needProjectId } from "../projectUtils";
 import { logLabeledBullet, logLabeledWarning, logLabeledSuccess } from "../utils";
 import * as errors from "./errors";
+import { Message } from '@google-cloud/pubsub';
 
 export async function diffSchema(schema: Schema): Promise<Diff[]> {
   const { serviceName, instanceName, databaseId } = getIdentifiers(schema);
-  await ensureServiceIsConnectedToCloudSql(serviceName, instanceName, databaseId);
+  await ensureServiceIsConnectedToCloudSql(
+    serviceName,
+    instanceName,
+    databaseId,
+    /* linkIfNotConnected=*/ false,
+  );
   try {
     await upsertSchema(schema, /** validateOnly=*/ true);
+    logLabeledSuccess("dataconnect", `Database schema is up to date.`);
   } catch (err: any) {
+    if (err.status !== 400) {
+      throw err;
+    }
+    // Display failed precondition errors nicely.
     const invalidConnectors = errors.getInvalidConnectors(err);
     if (invalidConnectors.length) {
       displayInvalidConnectors(invalidConnectors);
@@ -29,7 +40,6 @@ export async function diffSchema(schema: Schema): Promise<Diff[]> {
       return incompatible.diffs;
     }
   }
-  logLabeledSuccess("dataconnect", `Database schema is up to date.`);
   return [];
 }
 
@@ -42,11 +52,20 @@ export async function migrateSchema(args: {
   const { options, schema, allowNonInteractiveMigration, validateOnly } = args;
 
   const { serviceName, instanceId, instanceName, databaseId } = getIdentifiers(schema);
-  await ensureServiceIsConnectedToCloudSql(serviceName, instanceName, databaseId);
+  await ensureServiceIsConnectedToCloudSql(
+    serviceName,
+    instanceName,
+    databaseId,
+    /* linkIfNotConnected=*/ true,
+  );
   try {
     await upsertSchema(schema, validateOnly);
     logger.debug(`Database schema was up to date for ${instanceId}:${databaseId}`);
   } catch (err: any) {
+    if (err.status !== 400) {
+      throw err;
+    }
+    // Parse and handle failed precondition errors, then retry.
     const incompatible = errors.getIncompatibleSchemaError(err);
     const invalidConnectors = errors.getInvalidConnectors(err);
     if (!incompatible && !invalidConnectors.length) {
@@ -266,37 +285,43 @@ function displayInvalidConnectors(invalidConnectors: string[]) {
 
 // If a service has never had a schema with schemaValidation=strict
 // (ie when users create a service in console),
-// the backend will not have the necesary permissions to check cSQL for differences.
+// the backend will not have the necessary permissions to check cSQL for differences.
 // We fix this by upserting the currently deployed schema with schemaValidation=strict,
 async function ensureServiceIsConnectedToCloudSql(
   serviceName: string,
   instanceId: string,
   databaseId: string,
+  linkIfNotConnected: boolean,
 ) {
   let currentSchema: Schema;
   try {
     currentSchema = await getSchema(serviceName);
   } catch (err: any) {
-    if (err.status === 404) {
-      // If no schema has been deployed yet, deploy an empty one to get connectivity.
-      currentSchema = {
-        name: `${serviceName}/schemas/${SCHEMA_ID}`,
-        source: {
-          files: [],
-        },
-        primaryDatasource: {
-          postgresql: {
-            database: databaseId,
-            cloudSql: {
-              instance: instanceId,
-            },
-          },
-        },
-      };
-      logLabeledBullet("dataconnect", `Linking the Cloud SQL instance...`);
-    } else {
+    if (err.status !== 404) {
       throw err;
     }
+    if (!linkIfNotConnected) {
+      logLabeledWarning("dataconnect", `Not yet linked to the Cloud SQL instance.`);
+      return;
+    }
+    // TODO: make this prompt
+    // Should we upsert service here as well? so `database:sql:migrate` work for new service as well.
+    logLabeledBullet("dataconnect", `Linking the Cloud SQL instance...`);
+    // If no schema has been deployed yet, deploy an empty one to get connectivity.
+    currentSchema = {
+      name: `${serviceName}/schemas/${SCHEMA_ID}`,
+      source: {
+        files: [],
+      },
+      primaryDatasource: {
+        postgresql: {
+          database: databaseId,
+          cloudSql: {
+            instance: instanceId,
+          },
+        },
+      },
+    };
   }
   const postgresql = currentSchema.primaryDatasource.postgresql;
   if (postgresql?.cloudSql.instance !== instanceId) {

src/gcp/cloudsql/cloudsqladmin.ts

@@ -51,8 +51,6 @@ export async function createInstance(
       userLabels: { "firebase-data-connect": "ft" },
       insightsConfig: {
         queryInsightsEnabled: true,
-        queryPlansPerMinute: 5, // Match the default settings
-        queryStringLength: 1024, // Match the default settings
       },
     },
   });

src/gcp/cloudsql/types.ts

@@ -56,8 +56,6 @@ export interface DatabaseFlag {
 
 interface InsightsConfig {
   queryInsightsEnabled: boolean;
-  queryPlansPerMinute: number;
-  queryStringLength: number;
 }
 
 // TODO: Consider splitting off return only fields and input fields into different types.