Refactor deletes in direct deployment by denik · Pull Request #3163 · databricks/cli

denik · 2025-07-01T17:16:25Z

Changes

Remove DoDelete method from IResource, add a static function per resource.

Why

Delete operation is special - it does not require config. In fact, it is often run when config is gone.

On the other hand all other operations need config.

To support both delete and non-delete operation in one class we would need to have constant checks/asserts config != nil. To avoid this complication, I'm taking delete out of IResource.

Tests

Existing tests.

eng-dev-ecosystem-bot · 2025-07-01T17:25:51Z

Run: 16021370325

	Env	✅‌pass	❌‌FAIL	💥‌PANIC	🔄‌flaky	🙈‌skip	🤯‌MISS
❌‌	aws linux	244	21	2		371	29
❌‌	aws windows	257	13	2		354	41
🔄‌	aws-ucws linux	398			1	272
🔄‌	azure linux	294			3	371
💥‌	azure windows	288		1		370	9
✅‌	azure-ucws linux	401				269
🔄‌	azure-ucws windows	400			2	268
🔄‌	gcp linux	291			5	373
✅‌	gcp windows	297				372

76 failing tests:

Test Name	aws linux	aws windows	aws-ucws linux	azure linux	azure windows	azure-ucws windows	gcp linux
TestAccept	🤯‌MISS	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/deploy/files/no-snapshot-sync	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/deploy/files/no-snapshot-sync/DATABRICKS_CLI_DEPLOYMENT=direct-exp	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/deploy/files/no-snapshot-sync/DATABRICKS_CLI_DEPLOYMENT=terraform	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/deploy/jobs/fail-on-active-runs	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/deploy/jobs/fail-on-active-runs/DATABRICKS_CLI_DEPLOYMENT=direct-exp	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/deploy/jobs/fail-on-active-runs/DATABRICKS_CLI_DEPLOYMENT=terraform	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/deploy/jobs/shared-root-path	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/deploy/jobs/shared-root-path/DATABRICKS_CLI_DEPLOYMENT=direct-exp	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/deploy/jobs/shared-root-path/DATABRICKS_CLI_DEPLOYMENT=terraform	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/local_state_staleness	🤯‌MISS	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/local_state_staleness/DATABRICKS_CLI_DEPLOYMENT=direct-exp	🤯‌MISS	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/local_state_staleness/DATABRICKS_CLI_DEPLOYMENT=terraform	🤯‌MISS	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/resources/pipelines	🤯‌MISS	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/resources/pipelines/DATABRICKS_CLI_DEPLOYMENT=direct-exp	🤯‌MISS	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/resources/pipelines/DATABRICKS_CLI_DEPLOYMENT=terraform	❌‌FAIL	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/templates/default-python/combinations/classic	🤯‌MISS	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/templates/default-python/combinations/classic/DATABRICKS_CLI_DEPLOYMENT=direct-exp/DLT=no/NBOOK=yes/PY=no	❌‌FAIL	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/templates/default-python/combinations/classic/DATABRICKS_CLI_DEPLOYMENT=direct-exp/DLT=yes/NBOOK=no/PY=yes	❌‌FAIL	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/templates/default-python/combinations/classic/DATABRICKS_CLI_DEPLOYMENT=direct-exp/DLT=yes/NBOOK=yes/PY=yes	❌‌FAIL	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/templates/default-python/combinations/classic/DATABRICKS_CLI_DEPLOYMENT=terraform/DLT=no/NBOOK=no/PY=no	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/templates/default-python/combinations/classic/DATABRICKS_CLI_DEPLOYMENT=terraform/DLT=no/NBOOK=no/PY=yes	❌‌FAIL	❌‌FAIL	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/templates/default-python/combinations/classic/DATABRICKS_CLI_DEPLOYMENT=terraform/DLT=no/NBOOK=yes/PY=no	❌‌FAIL	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/templates/default-python/combinations/classic/DATABRICKS_CLI_DEPLOYMENT=terraform/DLT=no/NBOOK=yes/PY=yes	❌‌FAIL	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/templates/default-python/combinations/classic/DATABRICKS_CLI_DEPLOYMENT=terraform/DLT=yes/NBOOK=no/PY=no	🤯‌MISS	❌‌FAIL	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/templates/default-python/combinations/classic/DATABRICKS_CLI_DEPLOYMENT=terraform/DLT=yes/NBOOK=no/PY=yes	🤯‌MISS	❌‌FAIL	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/templates/default-python/combinations/classic/DATABRICKS_CLI_DEPLOYMENT=terraform/DLT=yes/NBOOK=yes/PY=no	💥‌PANIC	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/templates/default-python/combinations/serverless	✅‌pass	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/templates/default-python/combinations/serverless/DATABRICKS_CLI_DEPLOYMENT=direct-exp/DLT=no/NBOOK=no/PY=no	🙈‌skip	🤯‌MISS	✅‌pass	🙈‌skip	🙈‌skip	✅‌pass	🙈‌skip
TestAccept/bundle/templates/default-python/combinations/serverless/DATABRICKS_CLI_DEPLOYMENT=direct-exp/DLT=no/NBOOK=no/PY=yes	🙈‌skip	🤯‌MISS	✅‌pass	🙈‌skip	🙈‌skip	✅‌pass	🙈‌skip
TestAccept/bundle/templates/default-python/combinations/serverless/DATABRICKS_CLI_DEPLOYMENT=direct-exp/DLT=no/NBOOK=yes/PY=no	🙈‌skip	🤯‌MISS	✅‌pass	🙈‌skip	🙈‌skip	✅‌pass	🙈‌skip
TestAccept/bundle/templates/default-python/combinations/serverless/DATABRICKS_CLI_DEPLOYMENT=direct-exp/DLT=no/NBOOK=yes/PY=yes	🙈‌skip	🤯‌MISS	✅‌pass	🙈‌skip	🙈‌skip	✅‌pass	🙈‌skip
TestAccept/bundle/templates/default-python/combinations/serverless/DATABRICKS_CLI_DEPLOYMENT=direct-exp/DLT=yes/NBOOK=no/PY=no	🙈‌skip	🤯‌MISS	✅‌pass	🙈‌skip	🙈‌skip	✅‌pass	🙈‌skip
TestAccept/bundle/templates/default-python/combinations/serverless/DATABRICKS_CLI_DEPLOYMENT=direct-exp/DLT=yes/NBOOK=no/PY=yes	🙈‌skip	🤯‌MISS	✅‌pass	🙈‌skip	🙈‌skip	✅‌pass	🙈‌skip
TestAccept/bundle/templates/default-python/combinations/serverless/DATABRICKS_CLI_DEPLOYMENT=direct-exp/DLT=yes/NBOOK=yes/PY=no	🙈‌skip	🤯‌MISS	✅‌pass	🙈‌skip	🙈‌skip	✅‌pass	🙈‌skip
TestAccept/bundle/templates/default-python/combinations/serverless/DATABRICKS_CLI_DEPLOYMENT=direct-exp/DLT=yes/NBOOK=yes/PY=yes	🙈‌skip	🤯‌MISS	✅‌pass	🙈‌skip	🙈‌skip	✅‌pass	🙈‌skip
TestAccept/bundle/templates/default-python/combinations/serverless/DATABRICKS_CLI_DEPLOYMENT=terraform/DLT=no/NBOOK=no/PY=no	🙈‌skip	🤯‌MISS	✅‌pass	🙈‌skip	🙈‌skip	✅‌pass	🙈‌skip
TestAccept/bundle/templates/default-python/combinations/serverless/DATABRICKS_CLI_DEPLOYMENT=terraform/DLT=no/NBOOK=no/PY=yes	🙈‌skip	🤯‌MISS	✅‌pass	🙈‌skip	🙈‌skip	✅‌pass	🙈‌skip
TestAccept/bundle/templates/default-python/combinations/serverless/DATABRICKS_CLI_DEPLOYMENT=terraform/DLT=no/NBOOK=yes/PY=no	🙈‌skip	🤯‌MISS	✅‌pass	🙈‌skip	🙈‌skip	✅‌pass	🙈‌skip
TestAccept/bundle/templates/default-python/combinations/serverless/DATABRICKS_CLI_DEPLOYMENT=terraform/DLT=no/NBOOK=yes/PY=yes	🙈‌skip	🤯‌MISS	✅‌pass	🙈‌skip	🙈‌skip	✅‌pass	🙈‌skip
TestAccept/bundle/templates/default-python/combinations/serverless/DATABRICKS_CLI_DEPLOYMENT=terraform/DLT=yes/NBOOK=no/PY=no	🙈‌skip	🤯‌MISS	✅‌pass	🙈‌skip	🙈‌skip	✅‌pass	🙈‌skip
TestAccept/bundle/templates/default-python/combinations/serverless/DATABRICKS_CLI_DEPLOYMENT=terraform/DLT=yes/NBOOK=no/PY=yes	🙈‌skip	🤯‌MISS	✅‌pass	🙈‌skip	🙈‌skip	✅‌pass	🙈‌skip
TestAccept/bundle/templates/default-python/combinations/serverless/DATABRICKS_CLI_DEPLOYMENT=terraform/DLT=yes/NBOOK=yes/PY=no	🙈‌skip	🤯‌MISS	✅‌pass	🙈‌skip	🙈‌skip	✅‌pass	🙈‌skip
TestAccept/bundle/templates/default-python/combinations/serverless/DATABRICKS_CLI_DEPLOYMENT=terraform/DLT=yes/NBOOK=yes/PY=yes	🙈‌skip	🤯‌MISS	✅‌pass	🙈‌skip	🙈‌skip	✅‌pass	🙈‌skip
TestAccept/bundle/templates/default-python/integration_classic	✅‌pass	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/templates/default-python/integration_classic/DATABRICKS_CLI_DEPLOYMENT=terraform/UV_PYTHON=3.10	✅‌pass	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/templates/default-python/integration_classic/DATABRICKS_CLI_DEPLOYMENT=terraform/UV_PYTHON=3.11	✅‌pass	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/templates/default-python/integration_classic/DATABRICKS_CLI_DEPLOYMENT=terraform/UV_PYTHON=3.12	✅‌pass	🤯‌MISS	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/templates/default-python/integration_classic/DATABRICKS_CLI_DEPLOYMENT=terraform/UV_PYTHON=3.13	✅‌pass	💥‌PANIC	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/bundle/templates/default-python/integration_classic/DATABRICKS_CLI_DEPLOYMENT=terraform/UV_PYTHON=3.9	✅‌pass	❌‌FAIL	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/selftest/record_cloud/workspace-file-io	❌‌FAIL	❌‌FAIL	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/selftest/record_cloud/workspace-file-io/DATABRICKS_CLI_DEPLOYMENT=direct-exp	✅‌pass	❌‌FAIL	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestAccept/selftest/record_cloud/workspace-file-io/DATABRICKS_CLI_DEPLOYMENT=terraform	❌‌FAIL	❌‌FAIL	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestFetchRepositoryInfoAPI_FromRepo	❌‌FAIL	❌‌FAIL	✅‌pass	✅‌pass	✅‌pass	✅‌pass	🔄‌flaky
TestFilerReadWrite	❌‌FAIL	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestFilerReadWrite/workspace_files	❌‌FAIL	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestInprocessMode	🤯‌MISS	🤯‌MISS	🙈‌skip	🙈‌skip	🙈‌skip	🙈‌skip	🙈‌skip
TestLock	❌‌FAIL	❌‌FAIL	✅‌pass	✅‌pass	✅‌pass	✅‌pass	🔄‌flaky
TestReposCreateWithProvider	❌‌FAIL	❌‌FAIL	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestReposCreateWithoutProvider	❌‌FAIL	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestReposDeleteByID	❌‌FAIL	❌‌FAIL	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestReposDeleteByPath	❌‌FAIL	❌‌FAIL	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestReposGet	❌‌FAIL	❌‌FAIL	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestReposUpdate	❌‌FAIL	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestSyncEnsureRemotePathIsUsableIfRepoDoesntExist	🤯‌MISS	🤯‌MISS	✅‌pass	✅‌pass	🤯‌MISS	✅‌pass	✅‌pass
TestSyncEnsureRemotePathIsUsableIfRepoExists	🤯‌MISS	🤯‌MISS	✅‌pass	✅‌pass	🤯‌MISS	✅‌pass	✅‌pass
TestSyncEnsureRemotePathIsUsableInWorkspace	🤯‌MISS	🤯‌MISS	✅‌pass	✅‌pass	🤯‌MISS	✅‌pass	✅‌pass
TestSyncFullFileSync	❌‌FAIL	✅‌pass	🔄‌flaky	✅‌pass	✅‌pass	✅‌pass	✅‌pass
TestSyncIncrementalFileOverwritesFolder	🤯‌MISS	🤯‌MISS	✅‌pass	🔄‌flaky	🤯‌MISS	✅‌pass	✅‌pass
TestSyncIncrementalFileSync	❌‌FAIL	✅‌pass	✅‌pass	🔄‌flaky	✅‌pass	🔄‌flaky	🔄‌flaky
TestSyncIncrementalSyncFileToPythonNotebook	🤯‌MISS	🤯‌MISS	✅‌pass	✅‌pass	🤯‌MISS	✅‌pass	🔄‌flaky
TestSyncIncrementalSyncPythonNotebookDelete	🤯‌MISS	🤯‌MISS	✅‌pass	✅‌pass	🤯‌MISS	✅‌pass	✅‌pass
TestSyncIncrementalSyncPythonNotebookToFile	🤯‌MISS	🤯‌MISS	✅‌pass	✅‌pass	🤯‌MISS	✅‌pass	✅‌pass
TestSyncNestedFolderDoesntFailOnNonEmptyDirectory	🤯‌MISS	🤯‌MISS	✅‌pass	🔄‌flaky	🤯‌MISS	✅‌pass	🔄‌flaky
TestSyncNestedFolderSync	💥‌PANIC	💥‌PANIC	✅‌pass	✅‌pass	💥‌PANIC	✅‌pass	✅‌pass
TestSyncNestedSpacePlusAndHashAreEscapedSync	🤯‌MISS	🤯‌MISS	✅‌pass	✅‌pass	🤯‌MISS	🔄‌flaky	✅‌pass

pietern

Taking this a step further, why do the resource wrappers store the config to begin with?

It can be a bag of functions for every resource group where the functions accept the config as a parameter.

denik · 2025-07-02T09:28:50Z

Taking this a step further, why do the resource wrappers store the config to begin with?

If those methods accept any we lose type safety we have today. If those methods accept specific type then we need to use reflection to call it. Currently we use regular interfaces and limit reflection to the constructor.

denik requested review from andrewnester, anton-107, pietern and shreyas-goenka as code owners July 1, 2025 17:16

denik temporarily deployed to test-trigger-is July 1, 2025 17:16 — with GitHub Actions Inactive

denik temporarily deployed to test-trigger-is July 2, 2025 07:07 — with GitHub Actions Inactive

denik force-pushed the denik/direct-delete branch from 7b87e0c to c18609d Compare July 2, 2025 07:07

denik temporarily deployed to test-trigger-is July 2, 2025 07:08 — with GitHub Actions Inactive

pietern reviewed Jul 2, 2025

View reviewed changes

pietern approved these changes Jul 2, 2025

View reviewed changes

denik added 2 commits July 2, 2025 11:26

Refactor deletes in direct deployment

c23a679

constants for key names

f80eba8

denik force-pushed the denik/direct-delete branch from c18609d to f80eba8 Compare July 2, 2025 09:26

denik temporarily deployed to test-trigger-is July 2, 2025 09:26 — with GitHub Actions Inactive

denik merged commit f948985 into main Jul 2, 2025
12 of 13 checks passed

denik deleted the denik/direct-delete branch July 2, 2025 09:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor deletes in direct deployment#3163

Refactor deletes in direct deployment#3163
denik merged 2 commits intomainfrom
denik/direct-delete

denik commented Jul 1, 2025

Uh oh!

eng-dev-ecosystem-bot commented Jul 1, 2025 •

edited

Loading

Uh oh!

pietern left a comment •

edited

Loading

Uh oh!

denik commented Jul 2, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

denik commented Jul 1, 2025

Changes

Why

Tests

Uh oh!

eng-dev-ecosystem-bot commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pietern left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

denik commented Jul 2, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

eng-dev-ecosystem-bot commented Jul 1, 2025 •

edited

Loading

pietern left a comment •

edited

Loading