Add prompt when a pipeline recreation or deletion happens #1672
Conversation
This file contains 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
|
Triggered nighties on this PR... |
lennartkats-db
requested changes
Aug 23, 2024
lennartkats-db
requested changes
Aug 29, 2024
lennartkats-db
approved these changes
Sep 2, 2024
github-merge-queue bot
pushed a commit
that referenced
this pull request
Sep 2, 2024
## 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 ```
|
Trigger nightlies again... |
|
Nightlies are green on this PR. |
pietern
approved these changes
Sep 4, 2024
|
|
||
| // Recreating DLT pipeline leads to metadata loss and for a transient period | ||
| // the underling tables will be unavailable. | ||
| return actions.Replace() || actions.Delete() |
There was a problem hiding this comment.
Isn't this always true per the switch/case on the action type in parseTerraformActions?
There was a problem hiding this comment.
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.
| 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:` |
There was a problem hiding this comment.
The note about recreation should only be shown on actual recreation.
Can be a follow-up.
There was a problem hiding this comment.
Ack. Will do.
shreyas-goenka
changed the title
andrewnester
added a commit
that referenced
this pull request
Sep 5, 2024
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)).
github-merge-queue bot
pushed a commit
that referenced
this pull request
Sep 5, 2024
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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
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-approveflag.Tests
Manually, and new unit and integration tests.