-
Notifications
You must be signed in to change notification settings - Fork 267
azd env remove for removing local environment files
#6511
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR adds a new azd env remove command to delete local environment configuration files without removing Azure resources. The command provides both positional and flag-based environment selection, includes confirmation prompts (skippable with --force), and supports the rm alias.
Changes:
- New
azd env removecommand withrmalias for deleting local environment files - Support for both positional argument and
-eflag to specify environment name - Confirmation prompt with
--forceflag to skip - Comprehensive functional tests covering various usage scenarios
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| cli/azd/cmd/env_remove.go | New command implementation with validation, confirmation, and deletion logic |
| cli/azd/cmd/env.go | Command registration and unrelated change to envGetValuesAction |
| cli/azd/test/functional/env_test.go | Functional tests for remove command with multiple scenarios |
| cli/azd/cmd/testdata/TestUsage-azd-env-remove.snap | Snapshot test for command help text |
| cli/azd/pkg/environment/manager.go | Debug print statement added (appears to be leftover) |
a76cd21 to
efb9ad2
Compare
| ctx, | ||
| input.ConsoleOptions{ | ||
| Message: fmt.Sprintf( | ||
| "Remove the environment '%s'?", env.Name), |
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.
Consider enhancing this message to communicate that:
- environment can't be recovered/undeleted after this.
- Any resource in Azure linked to this env is not deleted
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.
I get the suggestion. Does the message printout above feel sufficient?
My UX preference here is for prompt messages to be short and concise.
Something like "Remove the environment 'dev'? This will not delete any Azure resources" may dilute the prompt confirmation, but I'm happy to hear about any examples you may have in mind.
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.
I would consider doing a check to see if this environment does contain any provisioned resources and if so warning the user. I agree that this command shouldn't remove them, but should let the user know that there will be orphaned resources that will need to be manually cleaned up.
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.
I agree that would be a nicer user experience as well. Immediately, it may be somewhat tricky to land.
The naive approach of calling State() takes a significant amount of time, since it involves compiling the infra, querying the deployment state, as I had noted in the issue. The intention is to indeed serve the notice, but the notice itself takes a long time to get around to, which in my mind degrades the perceived user experience.
Longer term, we may add the new confirmation prompt / warning. My vote is this would be a non-blocking future improvement. I logged #6522 as a placeholder issue to explore storing state to enable this scenario.
Note that deleting the resources after an "accident removal" using azd is still completely possible with azd env new with the same name, and choosing the subscription + location, before running azd down. This used to prompt the user for all parameters, but has been cleaned up with #6345, so I do like the overall story we are telling here today.
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.
I would only add the this action is irreversible to the wording.
And I would consider a variation to what Wallace mentioned. Instead of checking if the env was provisioned before. I would only look at the number of keys defined in the .env and the keys in the config.json and mention that in the wording with a warning when the number of keys is greater than 3 (assuming location, sub and env-name the 3 based keys) , or if the number of keys after removing the known keys is greater than 0.
Then I would print:
"This action will delete permanently [and saved parameters. This will be irreversible"
Those numbers should be super fast to get
efb9ad2 to
807c8b8
Compare
wbreza
left a comment
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.
Looks good - just a couple question/observations.
| ctx, | ||
| input.ConsoleOptions{ | ||
| Message: fmt.Sprintf( | ||
| "Remove the environment '%s'?", env.Name), |
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.
I would consider doing a check to see if this environment does contain any provisioned resources and if so warning the user. I agree that this command shouldn't remove them, but should let the user know that there will be orphaned resources that will need to be manually cleaned up.
807c8b8 to
63a83d0
Compare
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
azd env removeNew command to delete local environment config (
.azure/<env>/).--forceto skip confirmation. Does not delete Azure resources. Runazd downfirst if needed.Fixes #937