Move to a single prompt during bundle destroy by shreyas-goenka · Pull Request #1583 · databricks/cli

shreyas-goenka · 2024-07-09T17:37:27Z

Changes

Right now we ask users for two confirmations when destroying a bundle. One to destroy the resources and one to delete the files. This PR consolidates the two prompts into one.

Tests

Manually

Destroying a bundle with no resources:

➜ bundle-playground git:(master) ✗ cli bundle destroy
All files and directories at the following location will be deleted: /Users/shreyas.goenka@databricks.com/.bundle/bundle-playground/default
Would you like to proceed? [y/n]: y
No resources to destroy
Updating deployment state...
Deleting files...
Destroy complete!

Destroying a bundle with no remote state:

➜ bundle-playground git:(master) ✗ cli bundle destroy
No active deployment found to destroy!

When a user cancells a deployment:

➜ bundle-playground git:(master) ✗ cli bundle destroy
The following resources will be deleted:
delete job job_1
delete job job_2
delete pipeline foo
All files and directories at the following location will be deleted: /Users/shreyas.goenka@databricks.com/.bundle/bundle-playground/default
Would you like to proceed? [y/n]: n
Destroy cancelled!

When a user destroys resources:

➜ bundle-playground git:(master) ✗ cli bundle destroy
The following resources will be deleted:
delete job job_1
delete job job_2
delete pipeline foo
All files and directories at the following location will be deleted: /Users/shreyas.goenka@databricks.com/.bundle/bundle-playground/default
Would you like to proceed? [y/n]: y
Updating deployment state...
Deleting files...
Destroy complete!

pietern · 2024-07-17T08:18:48Z

+	// Core destructive mutators for destroy. These require informed user consent.
+	destroyCore := bundle.Seq(
+ terraform.Destroy(),
+ terraform.StatePush(),


(not blocking because it's the same as it was) Why do we push an empty state prior to deleting it?

It seems related to keeping the state consistent before deleting files, to be robust against errors, before we delete root_path.
We used to release the lock before calling files.Delete(). I think it's okay to remove this, will send a followup PR.
ref:
https://github.com/databricks/cli/pull/300/files#diff-26a7ca1d9267b02564c3de57565eea8310ba2099c4df658fb902af54a36662cd
#300 (comment)

pietern · 2024-07-23T17:37:29Z

Thanks, LGTM. @andrewnester Could you review as well?

Bundles: * Add UUID function to bundle template functions ([#1612](#1612)). * Upgrade TF provider to 1.49.0 ([#1617](#1617)). * Upgrade TF provider to 1.49.1 ([#1626](#1626)). * Support multiple locations for diagnostics ([#1610](#1610)). * Split artifact cleanup into prepare step before build ([#1618](#1618)). * Move to a single prompt during bundle destroy ([#1583](#1583)). Internal: * Add tests for the Workspace API readahead cache ([#1605](#1605)). * Update Python dependencies before install when upgrading a labs project ([#1624](#1624)).

## Changes Following up #1583 (comment). We can skip pushing because right after `root_path` is deleted, making this a no-op effectively. ## Tests

## Changes Right now we ask users for two confirmations when destroying a bundle. One to destroy the resources and one to delete the files. This PR consolidates the two prompts into one. ## Tests Manually Destroying a bundle with no resources: ``` ➜ bundle-playground git:(master) ✗ cli bundle destroy All files and directories at the following location will be deleted: /Users/shreyas.goenka@databricks.com/.bundle/bundle-playground/default Would you like to proceed? [y/n]: y No resources to destroy Updating deployment state... Deleting files... Destroy complete! ``` Destroying a bundle with no remote state: ``` ➜ bundle-playground git:(master) ✗ cli bundle destroy No active deployment found to destroy! ``` When a user cancells a deployment: ``` ➜ bundle-playground git:(master) ✗ cli bundle destroy The following resources will be deleted: delete job job_1 delete job job_2 delete pipeline foo All files and directories at the following location will be deleted: /Users/shreyas.goenka@databricks.com/.bundle/bundle-playground/default Would you like to proceed? [y/n]: n Destroy cancelled! ``` When a user destroys resources: ``` ➜ bundle-playground git:(master) ✗ cli bundle destroy The following resources will be deleted: delete job job_1 delete job job_2 delete pipeline foo All files and directories at the following location will be deleted: /Users/shreyas.goenka@databricks.com/.bundle/bundle-playground/default Would you like to proceed? [y/n]: y Updating deployment state... Deleting files... Destroy complete! ```

Bundles: * Add UUID function to bundle template functions ([#1612](#1612)). * Upgrade TF provider to 1.49.0 ([#1617](#1617)). * Upgrade TF provider to 1.49.1 ([#1626](#1626)). * Support multiple locations for diagnostics ([#1610](#1610)). * Split artifact cleanup into prepare step before build ([#1618](#1618)). * Move to a single prompt during bundle destroy ([#1583](#1583)). Internal: * Add tests for the Workspace API readahead cache ([#1605](#1605)). * Update Python dependencies before install when upgrading a labs project ([#1624](#1624)).

## Changes Following up #1583 (comment). We can skip pushing because right after `root_path` is deleted, making this a no-op effectively. ## Tests

shreyas-goenka added 12 commits July 8, 2024 19:39

Return early in bundle destroy if no deployment exists
c40c81b

-
d953157

add test for the mutator
34c4d97

fix seq
32f120c

some cleanup
df24350

add tests for the break signal
bf49b5b

more coverage for assert root path
abbb891

Merge remote-tracking branch 'origin' into exit-early-destroy
ac28802

use if mutator instead
27b09ce

move if to bundle package
bc42096

add tests for the if mutator
bedfcda

Move to a single prompt during bundle destroy
217a7cd

shreyas-goenka changed the title ~~Consolidate destroy prompt~~ Move to a single prompt during bundle destroyJul 9, 2024

shreyas-goenka added 4 commits July 9, 2024 19:38

remove unnecessary flag
c9d5eb0

merge
1daeb30

-
4f15fe9

-
82fe433

shreyas-goenka marked this pull request as ready for review July 9, 2024 18:14

shreyas-goenka requested review from andrewnester, lennartkats-db and pietern July 9, 2024 18:15

shreyas-goenka mentioned this pull request Jul 9, 2024
Prompt user when bundle deploy is destroying or recreating resources #1584
Closed

pietern reviewed Jul 17, 2024
View reviewed changes

shreyas-goenka added 3 commits July 17, 2024 15:56

remove logs
0231e0d

add link to action enum:
d25d904

-
fb02ea6

shreyas-goenka requested a review from pietern July 17, 2024 14:20

fix build
c49e685

shreyas-goenka mentioned this pull request Jul 23, 2024
Add resource for UC schemas to DABs #1413
Merged

Merge remote-tracking branch 'origin' into consolidate-destroy-prompt
a059f68

pietern reviewed Jul 23, 2024
View reviewed changes

Comment threadbundle/deploy/terraform/destroy.go
Comment threadbundle/phases/destroy.go Outdated

shreyas-goenka added 2 commits July 24, 2024 13:01

debug statement when there are no resources to destroy
64baad9

use enum value
9167e15

andrewnester approved these changes Jul 24, 2024
View reviewed changes

shreyas-goenka added this pull request to the merge queue Jul 24, 2024

Merged via the queue into main with commit e6241e1Jul 24, 2024

shreyas-goenka deleted the consolidate-destroy-prompt branch July 24, 2024 13:13

andrewnester mentioned this pull request Jul 25, 2024
Release v0.224.1 #1627
Merged

shreyas-goenka mentioned this pull request Aug 11, 2024
Skip pushing Terraform state after destroy #1667
Merged

github-merge-queueBot pushed a commit that referenced this pull request Aug 12, 2024
Skip pushing Terraform state after destroy (#1667)
1b984b4
## Changes Following up #1583 (comment). We can skip pushing because right after `root_path` is deleted, making this a no-op effectively. ## Tests

denik pushed a commit that referenced this pull request May 20, 2026
Skip pushing Terraform state after destroy (#1667)
aec6eda
## Changes Following up #1583 (comment). We can skip pushing because right after `root_path` is deleted, making this a no-op effectively. ## Tests

Navigation Menu

Uh oh!

Move to a single prompt during bundle destroy - #1583

Move to a single prompt during bundle destroy#1583
shreyas-goenka merged 23 commits into
mainfrom
consolidate-destroy-prompt

shreyas-goenka commented Jul 9, 2024•
edited
Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pieternJul 17, 2024

Uh oh!

shreyas-goenkaJul 17, 2024

Uh oh!

Uh oh!

Uh oh!

pietern commented Jul 23, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

shreyas-goenka commented Jul 9, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Tests

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pieternJul 17, 2024

Choose a reason for hiding this comment

Uh oh!

shreyas-goenkaJul 17, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pietern commented Jul 23, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shreyas-goenka commented Jul 9, 2024•
edited
Loading