init #208
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ import { | |
| takeFirstOrNull, | ||
| } from "@ctrlplane/db"; | ||
| import * as schema from "@ctrlplane/db/schema"; | ||
| import { getEventsForTargetDeleted, handleEvent } from "@ctrlplane/events"; | ||
| import { | ||
| cancelOldReleaseJobTriggersOnJobDispatch, | ||
| createJobApprovals, | ||
|
|
@@ -477,12 +478,20 @@ export const targetRouter = createTRPCRouter({ | |
| ), | ||
| }) | ||
| .input(z.array(z.string().uuid())) | ||
| .mutation(async ({ ctx, input }) => { | ||
| const targets = await ctx.db.query.target.findMany({ | ||
| where: inArray(schema.target.id, input), | ||
| }); | ||
| const events = ( | ||
| await Promise.allSettled(targets.map(getEventsForTargetDeleted)) | ||
| ).flatMap((r) => (r.status === "fulfilled" ? r.value : [])); | ||
| await Promise.allSettled(events.map(handleEvent)); | ||
|
|
||
| return ctx.db | ||
| .delete(schema.target) | ||
| .where(inArray(schema.target.id, input)) | ||
| .returning(); | ||
| }), | ||
|
Comment on lines
+481
to
+494
Contributor
There was a problem hiding this comment. Ensure proper error handling when processing events during target deletion The current implementation uses Consider explicitly handling rejected promises to ensure failures are not silently ignored. Here's how you might modify the code: .mutation(async ({ ctx, input }) => {
const targets = await ctx.db.query.target.findMany({
where: inArray(schema.target.id, input),
});
- const events = (
- await Promise.allSettled(targets.map(getEventsForTargetDeleted))
- ).flatMap((r) => (r.status === "fulfilled" ? r.value : []));
+ const events = await Promise.all(
+ targets.map(async (target) => {
+ try {
+ return await getEventsForTargetDeleted(target);
+ } catch (error) {
+ // Handle or log the error appropriately
+ console.error(`Failed to get events for target ${target.id}:`, error);
+ throw error;
+ }
+ })
+ );
- await Promise.allSettled(events.map(handleEvent));
+ await Promise.all(
+ events.map(async (event) => {
+ try {
+ await handleEvent(event);
+ } catch (error) {
+ // Handle or log the error appropriately
+ console.error(`Failed to handle event for target deletion:`, error);
+ throw error;
+ }
+ })
+ );
return ctx.db
.delete(schema.target)
.where(inArray(schema.target.id, input))
.returning();
}),This approach ensures that any errors during event generation or handling are caught and can be handled appropriately, possibly aborting the deletion if necessary.
🛠️ Refactor suggestion Wrap database operations in a transaction for consistency The deletion process includes multiple database operations: fetching targets, handling events, and deleting targets. To maintain data consistency, especially in cases where an error might occur during event handling, it's advisable to wrap these operations in a transaction. Consider modifying the code to use a transaction: .mutation(async ({ ctx, input }) => {
+ return ctx.db.transaction(async (tx) => {
const targets = await tx.query.target.findMany({
where: inArray(schema.target.id, input),
});
const events = await Promise.all(
targets.map(async (target) => {
// Error handling as discussed previously
return await getEventsForTargetDeleted(target);
})
);
await Promise.all(
events.map(async (event) => {
// Error handling as discussed previously
await handleEvent(event);
})
);
return tx
.delete(schema.target)
.where(inArray(schema.target.id, input))
.returning();
+ });
}),Wrapping these operations in a transaction ensures that if any part of the process fails, all changes can be rolled back, maintaining the integrity of your data.
|
||
|
|
||
| metadataKeys: protectedProcedure | ||
| .meta({ | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| CREATE TABLE IF NOT EXISTS "event" ( | ||
| "id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL, | ||
| "action" text NOT NULL, | ||
| "payload" jsonb NOT NULL, | ||
| "created_at" timestamp with time zone DEFAULT now() NOT NULL | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE IF NOT EXISTS "hook" ( | ||
| "id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL, | ||
| "action" text NOT NULL, | ||
| "name" text NOT NULL, | ||
| "scope_type" text NOT NULL, | ||
| "scope_id" uuid NOT NULL | ||
| ); | ||
|
Comment on lines
+8
to
+14
Contributor
There was a problem hiding this comment. 🛠️ Refactor suggestion Enhance data integrity and query performance Consider the following improvements:
CREATE INDEX IF NOT EXISTS idx_hook_scope ON hook(scope_type, scope_id);
ALTER TABLE hook ADD CONSTRAINT chk_hook_scope_type
CHECK (scope_type IN ('environment', 'deployment', 'target'));
ALTER TABLE hook ADD CONSTRAINT unq_hook_name_scope
UNIQUE (name, scope_type, scope_id); |
||
| --> statement-breakpoint | ||
| CREATE TABLE IF NOT EXISTS "runhook" ( | ||
| "id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL, | ||
| "hook_id" uuid NOT NULL, | ||
| "runbook_id" uuid NOT NULL | ||
| ); | ||
| --> statement-breakpoint | ||
| DO $$ BEGIN | ||
| ALTER TABLE "runhook" ADD CONSTRAINT "runhook_hook_id_hook_id_fk" FOREIGN KEY ("hook_id") REFERENCES "public"."hook"("id") ON DELETE cascade ON UPDATE no action; | ||
| EXCEPTION | ||
| WHEN duplicate_object THEN null; | ||
| END $$; | ||
| --> statement-breakpoint | ||
| DO $$ BEGIN | ||
| ALTER TABLE "runhook" ADD CONSTRAINT "runhook_runbook_id_runbook_id_fk" FOREIGN KEY ("runbook_id") REFERENCES "public"."runbook"("id") ON DELETE cascade ON UPDATE no action; | ||
| EXCEPTION | ||
| WHEN duplicate_object THEN null; | ||
| END $$; | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling for database operations.
The database query could fail due to various reasons (connection issues, permissions, etc.). Consider wrapping it in a try-catch block to handle potential errors gracefully.
export const run = async () => { + try { const expiredEnvironments = await db .select() .from(SCHEMA.environment) .where(lte(SCHEMA.environment.expiresAt, new Date())); if (expiredEnvironments.length === 0) return; + } catch (error) { + console.error('Failed to fetch expired environments:', error); + throw error; + }