databricks / cli

Databricks CLI
Other
132 stars 50 forks source link

Use precomputed terraform plan for `bundle deploy` #1640

Closed shreyas-goenka closed 2 months ago

shreyas-goenka commented 2 months ago

Changes

With https://github.com/databricks/cli/pull/1413 we started to compute and partially print the plan if it contained deletion of UC schemas. This PR uses the precomputed plan to avoid double planning when actually doing the terraform plan.

This fixes a performance regression introduced in https://github.com/databricks/cli/pull/1413.

Tests

Tested manually.

  1. Verified bundle deployment still works and deploys resources.
  2. Verified that the precomputed plan is indeed being used by attaching a debugger and removing the plan file right before the terraform apply process is spawned and asserting that terraform apply fails because the plan is not found.
shreyas-goenka commented 2 months ago

Triggered a round of nightlies on this PR.

shreyas-goenka commented 2 months ago

Waiting for the nightlies to pass before merging.

shreyas-goenka commented 2 months ago

The test for deploying empty bundles failed. This is because we used to always run terraform apply, which generated a barebones tfstate, but now we do not run terraform apply if the plan is empty.

Fixed by handing the case of state file missing in the terraform.StatePush mutator.

Triggering another round of the nightlies.

shreyas-goenka commented 2 months ago

The night are green now.