Skip to content

Cleaning up logging in dataconnect:sql:setup #8380

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

Merged
merged 5 commits into from
Apr 1, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/commands/dataconnect-sql-grant.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { pickService } from "../dataconnect/fileUtils";
import { grantRoleToUserInSchema } from "../dataconnect/schemaMigration";
import { requireAuth } from "../requireAuth";
import { FirebaseError } from "../error";
import { fdcSqlRoleMap } from "../gcp/cloudsql/permissions_setup";
import { fdcSqlRoleMap } from "../gcp/cloudsql/permissionsSetup";

const allowedRoles = Object.keys(fdcSqlRoleMap);

Expand Down
2 changes: 1 addition & 1 deletion src/commands/dataconnect-sql-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { FirebaseError } from "../error";
import { requireAuth } from "../requireAuth";
import { requirePermissions } from "../requirePermissions";
import { ensureApis } from "../dataconnect/ensureApis";
import { setupSQLPermissions, getSchemaMetadata } from "../gcp/cloudsql/permissions_setup";
import { setupSQLPermissions, getSchemaMetadata } from "../gcp/cloudsql/permissionsSetup";
import { DEFAULT_SCHEMA } from "../gcp/cloudsql/permissions";
import { getIdentifiers, ensureServiceIsConnectedToCloudSql } from "../dataconnect/schemaMigration";
import { getIAMUser } from "../gcp/cloudsql/connect";
Expand Down
5 changes: 2 additions & 3 deletions src/dataconnect/schemaMigration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
setupSQLPermissions,
getSchemaMetadata,
SchemaSetupStatus,
} from "../gcp/cloudsql/permissions_setup";
} from "../gcp/cloudsql/permissionsSetup";
import { DEFAULT_SCHEMA, firebaseowner } from "../gcp/cloudsql/permissions";
import { promptOnce, confirm } from "../prompt";
import { logger } from "../logger";
Expand All @@ -27,7 +27,6 @@
import { logLabeledBullet, logLabeledWarning, logLabeledSuccess } from "../utils";
import { iamUserIsCSQLAdmin } from "../gcp/cloudsql/cloudsqladmin";
import * as cloudSqlAdminClient from "../gcp/cloudsql/cloudsqladmin";

import * as errors from "./errors";

async function setupSchemaIfNecessary(
Expand All @@ -49,7 +48,7 @@
/* silent=*/ true,
);
} else {
logger.info(
logger.debug(
`Detected schema "${schemaInfo.name}" is setup in ${schemaInfo.setupStatus} mode. Skipping Setup.`,
);
}
Expand All @@ -57,7 +56,7 @@
return schemaInfo.setupStatus;
}

export async function diffSchema(

Check warning on line 59 in src/dataconnect/schemaMigration.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing JSDoc comment
options: Options,
schema: Schema,
schemaValidation?: SchemaValidation,
Expand Down Expand Up @@ -88,8 +87,8 @@
} else {
logLabeledSuccess("dataconnect", `Database schema is compatible.`);
}
} catch (err: any) {

Check warning on line 90 in src/dataconnect/schemaMigration.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected any. Specify a different type
if (err?.status !== 400) {

Check warning on line 91 in src/dataconnect/schemaMigration.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .status on an `any` value
throw err;
}
const invalidConnectors = errors.getInvalidConnectors(err);
Expand Down Expand Up @@ -117,8 +116,8 @@
logLabeledBullet("dataconnect", `generating schema changes, including optional changes...`);
await upsertSchema(schema, /** validateOnly=*/ true);
logLabeledSuccess("dataconnect", `no additional optional changes`);
} catch (err: any) {

Check warning on line 119 in src/dataconnect/schemaMigration.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected any. Specify a different type
if (err?.status !== 400) {

Check warning on line 120 in src/dataconnect/schemaMigration.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .status on an `any` value
throw err;
}
const incompatible = errors.getIncompatibleSchemaError(err);
Expand All @@ -140,7 +139,7 @@
return diffs;
}

export async function migrateSchema(args: {

Check warning on line 142 in src/dataconnect/schemaMigration.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing JSDoc comment
options: Options;
schema: Schema;
/** true for `dataconnect:sql:migrate`, false for `deploy` */
Expand Down Expand Up @@ -169,8 +168,8 @@
try {
await upsertSchema(schema, validateOnly);
logger.debug(`Database schema was up to date for ${instanceId}:${databaseId}`);
} catch (err: any) {

Check warning on line 171 in src/dataconnect/schemaMigration.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected any. Specify a different type
if (err?.status !== 400) {

Check warning on line 172 in src/dataconnect/schemaMigration.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .status on an `any` value
throw err;
}
// Parse and handle failed precondition errors, then retry.
Expand Down Expand Up @@ -222,8 +221,8 @@
setSchemaValidationMode(schema, validationMode);
try {
await upsertSchema(schema, validateOnly);
} catch (err: any) {

Check warning on line 224 in src/dataconnect/schemaMigration.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected any. Specify a different type
if (err.status !== 400) {

Check warning on line 225 in src/dataconnect/schemaMigration.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .status on an `any` value
throw err;
}
// Parse and handle failed precondition errors, then retry.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import * as clc from "colorette";

import { Options } from "../../options";
import {
firebaseowner,
Expand All @@ -7,19 +9,19 @@ import {
writerRolePermissions,
readerRolePermissions,
defaultPermissions,
FIREBASE_SUPER_USER,
CLOUDSQL_SUPER_USER,
FIREBASE_SUPER_USER,
} from "./permissions";
import { iamUserIsCSQLAdmin } from "./cloudsqladmin";
import { setupIAMUsers } from "./connect";
import { logger } from "../../logger";
import { confirm } from "../../prompt";
import * as clc from "colorette";
import { FirebaseError } from "../../error";
import { needProjectNumber } from "../../projectUtils";
import { executeSqlCmdsAsIamUser, executeSqlCmdsAsSuperUser, getIAMUser } from "./connect";
import { concat } from "lodash";
import { getDataConnectP4SA, toDatabaseUser } from "./connect";
import * as utils from "../../utils";

export type TableMetadata = {
name: string;
Expand Down Expand Up @@ -104,11 +106,14 @@ export async function setupSQLPermissions(
options: Options,
silent: boolean = false,
): Promise<SchemaSetupStatus.BrownField | SchemaSetupStatus.GreenField> {
const logFn = silent
? logger.debug
: (message: string) => {
return utils.logLabeledBullet("dataconnect", message);
};
const schema = schemaInfo.name;
// Step 0: Check current user can run setup and upsert IAM / P4SA users
logger.info(
`Detected schema "${schema}" setup status is ${schemaInfo.setupStatus}. Running setup...`,
);
logFn(`Detected schema "${schema}" setup status is ${schemaInfo.setupStatus}. Running setup...`);

const userIsCSQLAdmin = await iamUserIsCSQLAdmin(options);
if (!userIsCSQLAdmin) {
Expand All @@ -121,14 +126,14 @@ export async function setupSQLPermissions(
let runGreenfieldSetup = false;
if (schemaInfo.setupStatus === SchemaSetupStatus.GreenField) {
runGreenfieldSetup = true;
logger.info(
logFn(
`Database ${databaseId} has already been setup as greenfield project. Rerunning setup to repair any missing permissions.`,
);
}

if (schemaInfo.tables.length === 0) {
runGreenfieldSetup = true;
logger.info(`Found no tables in schema "${schema}", assuming greenfield project.`);
logFn(`Found no tables in schema "${schema}", assuming greenfield project.`);
}

// We need to setup the database
Expand All @@ -148,7 +153,7 @@ export async function setupSQLPermissions(
/** transaction=*/ true,
);

logger.info(clc.green("Database setup complete."));
logFn(clc.green("Database setup complete."));
return SchemaSetupStatus.GreenField;
}

Expand All @@ -158,7 +163,7 @@ export async function setupSQLPermissions(
);
}
const currentTablesOwners = [...new Set(schemaInfo.tables.map((t) => t.owner))];
logger.info(
logFn(
`We found some existing object owners [${currentTablesOwners.join(", ")}] in your cloudsql "${schema}" schema.`,
);

Expand All @@ -173,22 +178,22 @@ export async function setupSQLPermissions(

if (shouldSetupGreenfield) {
await setupBrownfieldAsGreenfield(instanceId, databaseId, schemaInfo, options, silent);
logger.info(clc.green("Database setup complete."));
logger.info(
logger.info(clc.green("Database setup complete.")); // If we do set up, always at least show this line.
logFn(
clc.yellow(
"IMPORTANT: please uncomment 'schemaValidation: \"COMPATIBLE\"' in your dataconnect.yaml file to avoid dropping any existing tables by mistake.",
),
);
return SchemaSetupStatus.GreenField;
} else {
logger.info(
logFn(
clc.yellow(
"Setting up database in brownfield mode.\n" +
`Note: SQL migrations can't be done through ${clc.bold("firebase dataconnect:sql:migrate")} in this mode.`,
),
);
await brownfieldSqlSetup(instanceId, databaseId, schemaInfo, options, silent);
logger.info(clc.green("Brownfield database setup complete."));
logFn(clc.green("Brownfield database setup complete."));
return SchemaSetupStatus.BrownField;
}
}
Expand Down Expand Up @@ -394,7 +399,7 @@ export async function brownfieldSqlSetup(
) {
const schema = schemaInfo.name;

// Step 1: Grant firebasesuperuser access to the original owner.
// Step 1: Grant firebasesuperuser access to the original owner
const uniqueTablesOwners = filterTableOwners(schemaInfo, databaseId);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

const grantOwnersToFirebasesuperuser = uniqueTablesOwners.map(
(owner) => `GRANT "${owner}" TO "${FIREBASE_SUPER_USER}"`,
Expand Down
4 changes: 2 additions & 2 deletions src/track.ts
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ function session(propertyName: GA4Property): AnalyticsSession | undefined {
const validateOnly = !!process.env.FIREBASE_CLI_MP_VALIDATE;
if (!usageEnabled() && propertyName !== "vscode") {
if (validateOnly) {
logger.warn("Google Analytics is DISABLED. To enable, (re)login and opt in to collection.");
logger.debug("Google Analytics is DISABLED. To enable, (re)login and opt in to collection.");
}
return;
}
Expand Down Expand Up @@ -352,7 +352,7 @@ function isDebugMode(): boolean {
if (account?.user.email.endsWith("@google.com")) {
try {
require("../tsconfig.json");
logger.info(
logger.debug(
`Using Google Analytics in DEBUG mode. Emulators (+ UI) events will be shown in GA Debug View only.`,
);
return true;
Expand Down
Loading