Skip to content

Switch to cascading deletes #8266

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 4 commits into from
Feb 27, 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
- Added initial delay when loading python functions (#8239)
- Enforced webframeworks enablement only on webframeworks sites (#8168)
- Fixed issue where `apps:init` throws an error upon app creation.
- Reenabled prompts for unused service deletion in `deploy --only`.
- Update Firebase Data Connect local toolkit to v1.8.3, which includes the following changes: (#8263)
- Adds a `_metadata.distance` field to vector similarity search results
- Fixes `auth` and `request.auth` when the request is unauthenticated
Expand Down
4 changes: 1 addition & 3 deletions scripts/dataconnect-test/tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
const FIREBASE_PROJECT = process.env.FBTOOLS_TARGET_PROJECT || "";
const FIREBASE_DEBUG = process.env.FIREBASE_DEBUG || "";

function expected(

Check warning on line 14 in scripts/dataconnect-test/tests.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing return type on function
serviceId: string,
databaseId: string,
schemaUpdateTime: string,
Expand All @@ -31,14 +31,12 @@
};
}

async function cleanUpService(projectId: string, serviceId: string, databaseId: string) {

Check warning on line 34 in scripts/dataconnect-test/tests.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing return type on function
await client.deleteServiceAndChildResources(
`projects/${projectId}/locations/us-central1/services/${serviceId}`,
);
await client.deleteService(`projects/${projectId}/locations/us-central1/services/${serviceId}`);
await deleteDatabase(projectId, "dataconnect-test", databaseId);
}

async function list() {

Check warning on line 39 in scripts/dataconnect-test/tests.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing return type on function
return await cli.exec(
"dataconnect:services:list",
FIREBASE_PROJECT,
Expand All @@ -51,7 +49,7 @@
);
}

async function migrate(force: boolean) {

Check warning on line 52 in scripts/dataconnect-test/tests.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing return type on function
const args = force ? ["--force"] : [];
if (FIREBASE_DEBUG) {
args.push("--debug");
Expand All @@ -68,7 +66,7 @@
);
}

async function deploy(force: boolean) {

Check warning on line 69 in scripts/dataconnect-test/tests.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing return type on function
const args = ["--only", "dataconnect"];
if (force) {
args.push("--force");
Expand All @@ -81,7 +79,7 @@
});
}

function toPath(p: string) {

Check warning on line 82 in scripts/dataconnect-test/tests.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing return type on function
return path.join(__dirname, p);
}

Expand Down Expand Up @@ -124,7 +122,7 @@
return { serviceId, databaseId };
}

function prepareStep(step: Step) {

Check warning on line 125 in scripts/dataconnect-test/tests.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing return type on function
fs.writeFileSync(toPath("fdc-test/schema/schema.gql"), step.schemaGQL, { mode: 420 /* 0o644 */ });
fs.writeFileSync(toPath("fdc-test/connector/connector.gql"), step.connectorGQL, {
mode: 420 /* 0o644 */,
Expand Down Expand Up @@ -158,8 +156,8 @@
await deploy(false);
await migrate(true);
await deploy(true);
} catch (err: any) {

Check warning on line 159 in scripts/dataconnect-test/tests.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected any. Specify a different type
expect(err.expectErr, `Unexpected error: ${err.message}`).to.be.true;

Check warning on line 160 in scripts/dataconnect-test/tests.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .expectErr on an `any` value

Check warning on line 160 in scripts/dataconnect-test/tests.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Invalid type "any" of template literal expression
}
expect(step.expectErr).to.be.false;
const result = await list();
Expand Down
87 changes: 0 additions & 87 deletions src/dataconnect/client.spec.ts

This file was deleted.

21 changes: 5 additions & 16 deletions src/dataconnect/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,11 @@ export async function createService(
return pollRes;
}

async function deleteService(serviceName: string): Promise<types.Service> {
// NOTE(fredzqm): Don't force delete yet. Backend would leave orphaned resources.
const op = await dataconnectClient().delete<types.Service>(serviceName);
export async function deleteService(serviceName: string): Promise<types.Service> {
// Note that we need to force delete in order to delete child resources too.
const op = await dataconnectClient().delete<types.Service>(serviceName, {
queryParams: { force: "true" },
});
const pollRes = await operationPoller.pollOperation<types.Service>({
apiOrigin: dataconnectOrigin(),
apiVersion: DATACONNECT_API_VERSION,
Expand All @@ -72,19 +74,6 @@ async function deleteService(serviceName: string): Promise<types.Service> {
return pollRes;
}

export async function deleteServiceAndChildResources(serviceName: string): Promise<void> {
const connectors = await listConnectors(serviceName);
await Promise.all(connectors.map(async (c) => deleteConnector(c.name)));
try {
await deleteSchema(serviceName);
} catch (err: any) {
if (err.status !== 404) {
throw err;
}
}
await deleteService(serviceName);
}

/** Schema methods */

export async function getSchema(serviceName: string): Promise<types.Schema | undefined> {
Expand Down
2 changes: 1 addition & 1 deletion src/dataconnect/schemaMigration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ async function promptForInvalidConnectorError(
!options.nonInteractive &&
(await confirm({
...options,
message: `Would you like to delete and recreate these connectors? This will cause ${clc.red(`downtime.`)}.`,
message: `Would you like to delete and recreate these connectors? This will cause ${clc.red(`downtime`)}.`,
}))
) {
return true;
Expand Down
39 changes: 17 additions & 22 deletions src/deploy/dataconnect/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { ResourceFilter } from "../../dataconnect/filters";
import { vertexAIOrigin } from "../../api";
import * as ensureApiEnabled from "../../ensureApiEnabled";
import { join } from "node:path";
import { confirm } from "../../prompt";

/**
* Checks for and creates a Firebase DataConnect service, if needed.
Expand Down Expand Up @@ -56,28 +57,22 @@ export default async function (
);

if (servicesToDelete.length) {
const warning = `The following services exist on ${projectId} but are not listed in your 'firebase.json'\n${servicesToDelete
.map((s) => s.name)
.join("\n")}\nConsider deleting these via the Firebase console if they are no longer needed.`;
utils.logLabeledWarning("dataconnect", warning);
// TODO: Switch this back to prompting for deletion.
// if (
// await confirm({
// force: options.force,
// nonInteractive: options.nonInteractive,
// message: `The following services exist on ${projectId} but are not listed in your 'firebase.json'\n${servicesToDelete
// .map((s) => s.name)
// .join("\n")}\nWould you like to delete these services?`,
// })
// ) {
// await Promise.all(
// servicesToDelete.map(async (s) => {
// const { projectId, locationId, serviceId } = splitName(s.name);
// await client.deleteService(projectId, locationId, serviceId);
// utils.logLabeledSuccess("dataconnect", `Deleted service ${s.name}`);
// }),
// );
// }
if (
await confirm({
force: options.force,
nonInteractive: options.nonInteractive,
message: `The following services exist on ${projectId} but are not listed in your 'firebase.json'\n${servicesToDelete
.map((s) => s.name)
.join("\n")}\nWould you like to delete these services?`,
})
) {
await Promise.all(
servicesToDelete.map(async (s) => {
await client.deleteService(s.name);
utils.logLabeledSuccess("dataconnect", `Deleted service ${s.name}`);
}),
);
}
}

// Provision CloudSQL resources
Expand Down
Loading