Add prompt when a pipeline recreation or deletion happens by shreyas-goenka · Pull Request #1672 · databricks/cli

shreyas-goenka · 2024-08-12T14:08:37Z

Changes

DLT pipeline recreations are destructive. They can lead to lost history of previous updates, outage of the tables temporarily and are potentially computationally expensive. Thus we make a breaking change where a prompt is shown to the user if there configuration changes will lead to a DLT recreation.

Users can skip the prompt by specifying the --auto-approve flag.

Tests

Manually, and new unit and integration tests.

➜ bundle-playground-3 cli bundle deploy
Uploading bundle files to /Users/63ec021d-b0c6-49c0-93a0-5123953a1cb2/.bundle/test/development/files...
The following DLT pipelines will be recreated. Underlying tables will be unavailable for a transient period until the newly recreated pipelines are run once successfully. History of previous pipeline update runs will be lost because of recreation:
recreate pipeline foo
Would you like to proceed? [y/n]: n
Deployment cancelled!

shreyas-goenka · 2024-08-12T14:27:37Z

Triggered nighties on this PR...

lennartkats-db

This looks great. Please check the remaining minor comment and failing test.

## Changes We were not using the readers and writers set in the test fixtures in the progress logger. This PR fixes that. It also modifies `TestAccAbortBind`, which was implicitly relying on the bug. I encountered this bug while working on #1672. ## Tests Manually. From non-tty: ``` Error: failed to bind the resource, err: This bind operation requires user confirmation, but the current console does not support prompting. Please specify --auto-approve if you would like to skip prompts and proceed. ``` From tty, bind works as expected. ``` Confirm import changes? Changes will be remotely applied only after running 'bundle deploy'. [y/n]: y Updating deployment state... Successfully bound databricks_pipeline with an id '9d2dedbb-f522-4503-96ba-4bc4d5bfa77d'. Run 'bundle deploy' to deploy changes to your workspace ```

shreyas-goenka · 2024-09-02T14:15:04Z

Trigger nightlies again...

shreyas-goenka · 2024-09-03T11:15:50Z

Nightlies are green on this PR.

pietern · 2024-09-04T10:44:00Z

+
+ // Recreating DLT pipeline leads to metadata loss and for a transient period
+ // the underling tables will be unavailable.
+ return actions.Replace() || actions.Delete()


Isn't this always true per the switch/case on the action type in parseTerraformActions?

Yes, you are correct. This structure is from when this PR was just for recreates of DLT. I can clean this up in a followup.

pietern · 2024-09-04T10:44:42Z

+This action will result in the deletion or recreation of the following DLT Pipelines along with the
+Streaming Tables (STs) and Materialized Views (MVs) managed by them. Recreating the Pipelines will
+restore the defined STs and MVs through full refresh. Note that recreation is necessary when pipeline
+properties such as the 'catalog' or 'storage' are changed:`


The note about recreation should only be shown on actual recreation.
Can be a follow-up.

Ack. Will do.

CLI: * Do not error if we cannot prompt for a profile in `auth login` ([#1745](#1745)). Bundles: * Pass along $AZURE_CONFIG_FILE to Terraform process ([#1734](#1734)). * Add prompt when a pipeline recreation happens ([#1672](#1672)). * Use materialized views in the default-sql template ([#1709](#1709)). * Update templates to latest LTS DBR ([#1715](#1715)). * Make lock optional in the JSON schema ([#1738](#1738)). * Do not suppress normalisation diagnostics for resolving variables ([#1740](#1740)). * Include a permissions section in all templates ([#1713](#1713)). * Fixed complex variables are not being correctly merged from include files ([#1746](#1746)). * Fixed variable override in target with full variable syntax ([#1749](#1749)). Internal: * Consider serverless clusters as compatible for Python wheel tasks ([#1733](#1733)). * PythonMutator: explain missing package error ([#1736](#1736)). * Add `dyn.Time` to box a timestamp with its original string value ([#1732](#1732)). * Fix streaming of stdout, stdin, stderr in cobra test runner ([#1742](#1742)). Dependency updates: * Bump github.com/Masterminds/semver/v3 from 3.2.1 to 3.3.0 ([#1741](#1741)).

CLI: * Do not error if we cannot prompt for a profile in `auth login` ([#1745](#1745)). Bundles: As of this release CLI will show a prompt is if there are configuration changes which will lead to a DLT recreation. Users can skip the prompt by specifying the `--auto-approve` flag * Pass along $AZURE_CONFIG_FILE to Terraform process ([#1734](#1734)). * Add prompt when a pipeline recreation happens ([#1672](#1672)). * Use materialized views in the default-sql template ([#1709](#1709)). * Update templates to latest LTS DBR ([#1715](#1715)). * Make lock optional in the JSON schema ([#1738](#1738)). * Do not suppress normalisation diagnostics for resolving variables ([#1740](#1740)). * Include a permissions section in all templates ([#1713](#1713)). * Fixed complex variables are not being correctly merged from include files ([#1746](#1746)). * Fixed variable override in target with full variable syntax ([#1749](#1749)). Internal: * Consider serverless clusters as compatible for Python wheel tasks ([#1733](#1733)). * PythonMutator: explain missing package error ([#1736](#1736)). * Add `dyn.Time` to box a timestamp with its original string value ([#1732](#1732)). * Fix streaming of stdout, stdin, stderr in cobra test runner ([#1742](#1742)). Dependency updates: * Bump github.com/Masterminds/semver/v3 from 3.2.1 to 3.3.0 ([#1741](#1741)). --------- Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>

## Changes We were not using the readers and writers set in the test fixtures in the progress logger. This PR fixes that. It also modifies `TestAccAbortBind`, which was implicitly relying on the bug. I encountered this bug while working on #1672. ## Tests Manually. From non-tty: ``` Error: failed to bind the resource, err: This bind operation requires user confirmation, but the current console does not support prompting. Please specify --auto-approve if you would like to skip prompts and proceed. ``` From tty, bind works as expected. ``` Confirm import changes? Changes will be remotely applied only after running 'bundle deploy'. [y/n]: y Updating deployment state... Successfully bound databricks_pipeline with an id '9d2dedbb-f522-4503-96ba-4bc4d5bfa77d'. Run 'bundle deploy' to deploy changes to your workspace ```

## Changes DLT pipeline recreations are destructive. They can lead to lost history of previous updates, outage of the tables temporarily and are potentially computationally expensive. Thus we make a breaking change where a prompt is shown to the user if there configuration changes will lead to a DLT recreation. Users can skip the prompt by specifying the `--auto-approve` flag. This PR also fixes an issue with our test runner where logs from the cmdio.Logger would not get propagated to the reader returned by our cobra test runner. ## Tests Manually, and new unit and integration tests. ``` ➜ bundle-playground-3 cli bundle deploy Uploading bundle files to /Users/63ec021d-b0c6-49c0-93a0-5123953a1cb2/.bundle/test/development/files... The following DLT pipelines will be recreated. Underlying tables will be unavailable for a transient period until the newly recreated pipelines are run once successfully. History of previous pipeline update runs will be lost because of recreation: recreate pipeline foo Would you like to proceed? [y/n]: n Deployment cancelled! ```

CLI: * Do not error if we cannot prompt for a profile in `auth login` ([#1745](#1745)). Bundles: As of this release CLI will show a prompt is if there are configuration changes which will lead to a DLT recreation. Users can skip the prompt by specifying the `--auto-approve` flag * Pass along $AZURE_CONFIG_FILE to Terraform process ([#1734](#1734)). * Add prompt when a pipeline recreation happens ([#1672](#1672)). * Use materialized views in the default-sql template ([#1709](#1709)). * Update templates to latest LTS DBR ([#1715](#1715)). * Make lock optional in the JSON schema ([#1738](#1738)). * Do not suppress normalisation diagnostics for resolving variables ([#1740](#1740)). * Include a permissions section in all templates ([#1713](#1713)). * Fixed complex variables are not being correctly merged from include files ([#1746](#1746)). * Fixed variable override in target with full variable syntax ([#1749](#1749)). Internal: * Consider serverless clusters as compatible for Python wheel tasks ([#1733](#1733)). * PythonMutator: explain missing package error ([#1736](#1736)). * Add `dyn.Time` to box a timestamp with its original string value ([#1732](#1732)). * Fix streaming of stdout, stdin, stderr in cobra test runner ([#1742](#1742)). Dependency updates: * Bump github.com/Masterminds/semver/v3 from 3.2.1 to 3.3.0 ([#1741](#1741)). --------- Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>

shreyas-goenka added 3 commits August 12, 2024 16:00

Add prompt when a pipeline recreation happens
105a3df

grammer nit
d270727

rename
27af70e

shreyas-goenka marked this pull request as ready for review August 12, 2024 14:30

shreyas-goenka requested review from andrewnester and pietern August 12, 2024 14:31

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

lennartkats-db requested changes Aug 23, 2024
View reviewed changes

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

shreyas-goenka added 2 commits August 27, 2024 20:08

Merge remote-tracking branch 'origin' into prompt-on-pipeline-recreate
a0bc111

add the why
d6c564d

shreyas-goenka requested a review from lennartkats-db August 27, 2024 18:25

lennartkats-db requested changes Aug 29, 2024
View reviewed changes

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

shreyas-goenka added 2 commits August 30, 2024 11:40

fix test
7e3a453

add multiline message
d9ed8eb

shreyas-goenka requested a review from lennartkats-db August 30, 2024 09:55

lennartkats-db approved these changes Sep 2, 2024
View reviewed changes

Comment threadinternal/bundle/deploy_test.go Outdated

shreyas-goenka mentioned this pull request Sep 2, 2024
Fix streaming of stdout, stdin, stderr in cobra test runner #1742
Merged

shreyas-goenka added 2 commits September 2, 2024 14:27

Merge remote-tracking branch 'origin' into prompt-on-pipeline-recreate
8d2c2ab

add test for pipeline delete as well
2f7fa97

Merge remote-tracking branch 'origin' into prompt-on-pipeline-recreate
ec413af

pietern approved these changes Sep 4, 2024
View reviewed changes

shreyas-goenka added this pull request to the merge queue Sep 4, 2024

Merged via the queue into main with commit a27c24aSep 4, 2024

shreyas-goenka deleted the prompt-on-pipeline-recreate branch September 4, 2024 11:20

shreyas-goenka changed the title ~~Add prompt when a pipeline recreation happens~~Add prompt when a pipeline recreation or deletion happensSep 4, 2024

andrewnester mentioned this pull request Sep 5, 2024
[Release] Release v0.228.0 #1752
Merged

Navigation Menu

Uh oh!

Add prompt when a pipeline recreation or deletion happens - #1672

Add prompt when a pipeline recreation or deletion happens#1672
shreyas-goenka merged 10 commits into
mainfrom
prompt-on-pipeline-recreate

shreyas-goenka commented Aug 12, 2024•
edited
Loading

Uh oh!

shreyas-goenka commented Aug 12, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lennartkats-db left a comment

Uh oh!

Uh oh!

shreyas-goenka commented Sep 2, 2024

Uh oh!

shreyas-goenka commented Sep 3, 2024

Uh oh!

pieternSep 4, 2024

Uh oh!

shreyas-goenkaSep 4, 2024

Uh oh!

pieternSep 4, 2024

Uh oh!

shreyas-goenkaSep 4, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

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

Uh oh!

Changes

Tests

Uh oh!

shreyas-goenka commented Aug 12, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lennartkats-db left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shreyas-goenka commented Sep 2, 2024

Uh oh!

shreyas-goenka commented Sep 3, 2024

Uh oh!

pieternSep 4, 2024

Choose a reason for hiding this comment

Uh oh!

shreyas-goenkaSep 4, 2024

Choose a reason for hiding this comment

Uh oh!

pieternSep 4, 2024

Choose a reason for hiding this comment

Uh oh!

shreyas-goenkaSep 4, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shreyas-goenka commented Aug 12, 2024•
edited
Loading