Repository navigation
feat(stack): allow repeatable --stack-id in stack destroy (#6964) - #7000
tirthraj01 wants to merge 2 commits into
Conversation
![depthfirst-app[bot]](https://avatars.githubusercontent.com/in/1021002?s=60&v=4){kind=small}
7ttp
left a comment
7ttp
left a comment
There was a problem hiding this comment.
super, thanks for this! 💚 a few things to address before this lands:
| yield* output.success("", { | ||
| destroyed: failures.length === 0, | ||
| stacks: successes.map(({ id, result }) => ({ id, ...result })), | ||
| failures: failures.map(({ id, error }) => ({ id, message: error.message })), | ||
| }); | ||
| } |
There was a problem hiding this comment.
could this emit one error envelope? failed destroys currently print two JSON documents MachineErrorContext could carry the partial results, as stack start does here?:
cli/apps/cli/src/commands/experimental/stack/start/start.handler.ts
Lines 722 to 728 in a0666ab
| yield* stackDestroy({ | ||
| ...f.flags, | ||
| stackId: [f.stack.id, second.id], | ||
| }).pipe(Effect.provide(f.layer)); |
There was a problem hiding this comment.
this passes an array directly, lets run it thru Command.runWith it would cover repeated flags too
tbh would also be worth covering a missing later ID and prefix/full-ID dedupe...
| Flag.withDescription("Destroy an existing stack by id or unique id prefix."), | ||
| Flag.optional, | ||
| Flag.withDescription( | ||
| "Destroy an existing stack by id or unique id prefix; repeat to select several.", |
There was a problem hiding this comment.
could you also update stack-commands.md and SIDE_EFFECTS.md ?
| const error = Option.getOrElse( | ||
| Exit.findErrorOption(exit), | ||
| () => | ||
| new StackCommandDestroyError({ | ||
| reason: "unknown", | ||
| message: `Failed to destroy stack ${targetId}`, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
ig we should we keep the original error here? instead of a generic failure
| }), | ||
| ); | ||
| failures.push({ id: targetId, error }); | ||
| yield* output.error(`Failed to destroy stack ${targetId}: ${error.message}`); |
There was a problem hiding this comment.
imo the task and final error already report this failure. so we prob can drop this extra error line on stdout as well..
This branch has not been deployed
2 participants
What kind of change does this PR introduce?
Feature
What is the current behavior?
supabase stack destroyonly accepts a single--stack-idstring argument. Cleaning up multiple temporary or local stacks requires executingstack destroymultiple times, prompting for confirmation on each run.Fixes #6964
What is the new behavior?
--stack-id: The--stack-idflag now accepts multiple values (e.g.supabase stack destroy --stack-id id1 --stack-id id2 --yes).--stackand--stack-idremain strictly mutually exclusive when single or multiple IDs are provided.--stack/--stack-idmutual exclusivity, and resilient loop error handling.Additional context
Tested against integration test fixtures:
bun --bun vitest run apps/cli/src/commands/experimental/stack/destroy/destroy.integration.test.ts apps/cli/src/commands/experimental/stack/stack.shared.integration.test.ts(25 passed)types:check,lint:check,fmt:check, andlint:effect:checkall pass with 0 errors.