Skip to content

direct: Add DoUpdateWithChanges#3901

Merged
shreyas-goenka merged 7 commits intomainfrom
do-update-with-changes
Nov 11, 2025
Merged

direct: Add DoUpdateWithChanges#3901
shreyas-goenka merged 7 commits intomainfrom
do-update-with-changes

Conversation

@shreyas-goenka
Copy link
Contributor

@shreyas-goenka shreyas-goenka commented Nov 10, 2025

Why

Useful for model serving endpoint where we need to call multiple API endpoints based on the fields that were modified or had a remote drift.

relevant TF code: https://github.com/databricks/terraform-provider-databricks/blob/5501f662eb0a8a31bd5f94ba9e9130288b9314a6/serving/resource_model_serving.go#L370

This reverts commit b9fb814.
This reverts commit 7522cce.
@shreyas-goenka shreyas-goenka marked this pull request as ready for review November 10, 2025 18:39
@eng-dev-ecosystem-bot
Copy link
Collaborator

eng-dev-ecosystem-bot commented Nov 10, 2025

Run: 19242238012

Env 🔄​flaky 💚​RECOVERED 🙈​SKIP ✅​pass 🙈​skip
💚​ aws linux 1 1 356 602
💚​ aws windows 1 1 357 601
💚​ aws-ucws linux 1 1 491 493
💚​ aws-ucws windows 1 1 492 492
💚​ azure linux 1 1 356 601
🔄​ azure windows 4 1 1 353 600
🔄​ azure-ucws linux 3 1 485 492
🔄​ azure-ucws windows 4 1 1 484 491
🔄​ gcp linux 6 1 1 349 603
🔄​ gcp windows 7 1 1 349 602
17 failing tests:
Test Name aws linux aws windows aws-ucws linux aws-ucws windows azure linux azure windows azure-ucws linux azure-ucws windows gcp linux gcp windows
TestAccept 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 🔄​f 💚​R 💚​R 💚​R
TestAccept/bundle/generate/auto-bind ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p 🔄​f ✅​p
TestAccept/bundle/local_state_staleness ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p 🔄​f
TestAccept/bundle/local_state_staleness/DATABRICKS_BUNDLE_ENGINE=direct ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p 🔄​f
TestAccept/bundle/resources/clusters/deploy/data_security_mode ✅​p ✅​p ✅​p ✅​p ✅​p 🔄​f 🔄​f 🔄​f 🔄​f ✅​p
TestAccept/bundle/resources/clusters/deploy/data_security_mode/DATABRICKS_BUNDLE_ENGINE=direct ✅​p ✅​p ✅​p ✅​p ✅​p 🔄​f 🔄​f 🔄​f ✅​p ✅​p
TestAccept/bundle/resources/clusters/deploy/data_security_mode/DATABRICKS_BUNDLE_ENGINE=terraform ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p 🔄​f ✅​p
TestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=direct ✅​p ✅​p ✅​p ✅​p ✅​p 🔄​f ✅​p 🔄​f ✅​p ✅​p
TestAccept/bundle/resources/clusters/deploy/update-after-create/DATABRICKS_BUNDLE_ENGINE=terraform ✅​p ✅​p ✅​p ✅​p ✅​p 🔄​f ✅​p 🔄​f ✅​p ✅​p
TestAccept/bundle/resources/jobs/fail-on-active-runs/DATABRICKS_BUNDLE_ENGINE=direct ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p 🔄​f
TestAccept/bundle/resources/jobs/fail-on-active-runs/DATABRICKS_BUNDLE_ENGINE=terraform ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p 🔄​f
TestAccept/bundle/run/app-with-job 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
TestAccept/bundle/templates/default-python/combinations/classic ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p 🔄​f 🔄​f
TestAccept/bundle/templates/default-python/combinations/classic/DATABRICKS_BUNDLE_ENGINE=direct/DLT=yes/NBOOK=yes/PY=no ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p 🔄​f
TestAccept/bundle/templates/default-python/combinations/classic/DATABRICKS_BUNDLE_ENGINE=terraform/DLT=yes/NBOOK=yes/PY=no ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p 🔄​f ✅​p
TestAccept/bundle/templates/default-python/combinations/serverless ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p 🔄​f
TestLock ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p ✅​p 🔄​f ✅​p

@shreyas-goenka shreyas-goenka added this pull request to the merge queue Nov 11, 2025
Copy link
Contributor

@pietern pietern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use a separate function vs always passing the changes to the update function?

Merged via the queue into main with commit 041207d Nov 11, 2025
13 checks passed
@shreyas-goenka shreyas-goenka deleted the do-update-with-changes branch November 11, 2025 10:36
@denik
Copy link
Contributor

denik commented Nov 11, 2025

Why use a separate function vs always passing the changes to the update function?

Reasonable question. We went with this because passing changes to doUpdate seems like anti-pattern given that have separate ClassifyChanges/FieldTriggers for analyzing changes. So the idea is to make such cases explicit.

It does feel like there are too many DoUpdate variants and some consolidation is in order.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants