diff --git a/.nextchanges/bundles/registered-model-rename.md b/.nextchanges/bundles/registered-model-rename.md new file mode 100644 index 00000000000..5cf62805994 --- /dev/null +++ b/.nextchanges/bundles/registered-model-rename.md @@ -0,0 +1 @@ +* Rename a registered model in place when its `name` changes, keeping its versions, instead of deleting and recreating it. Registered models inside a renamed catalog or schema now follow it to the new name; recreating them failed because they already exist there. ([#7379](https://github.com/databricks/cli/pull/7379)) diff --git a/acceptance/bundle/migrate/auto/registered-model-rename/output.txt b/acceptance/bundle/migrate/auto/registered-model-rename/output.txt index fbad32d54f3..4393108783a 100644 --- a/acceptance/bundle/migrate/auto/registered-model-rename/output.txt +++ b/acceptance/bundle/migrate/auto/registered-model-rename/output.txt @@ -2,18 +2,17 @@ === Replay the prior terraform deployment (captured from a real terraform run) >>> replay_tfstate.py terraform-requests.json terraform.tfstate -=== Rename the model (an immutable id field) and deploy on direct +=== Rename the model (an updatable id field) and deploy on direct >>> DATABRICKS_BUNDLE_ENGINE=direct [CLI] bundle deploy -Warn: migration to direct: registered_models.my_model: name differs between config ("themodel-[UNIQUE_NAME]-renamed") and terraform state ("themodel-[UNIQUE_NAME]"); this resource will be recreated. +Warn: migration to direct: registered_models.my_model: name differs between config ("themodel-[UNIQUE_NAME]-renamed") and terraform state ("themodel-[UNIQUE_NAME]"); this resource will be renamed. Notice: migrating your bundle to direct deployment engine (https://github.com/databricks/cli/issues/6765). Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/[UNIQUE_NAME]/files... Migrated 2 resources to direct deployment engine. -Recreated registered_models.my_model +Updated registered_models.my_model Files: 2 uploaded, 0 deleted -Resources: 1 created, 0 changed, 1 deleted, 1 unchanged +Resources: 0 created, 1 changed, 0 deleted, 1 unchanged >>> print_migration_telemetry -direct_migrate_recreate_planned true direct_migrated_via_env true === Redeploy on direct: the model now matches config, so the plan is a clean no-op diff --git a/acceptance/bundle/migrate/auto/registered-model-rename/script b/acceptance/bundle/migrate/auto/registered-model-rename/script index 0b81939d6d4..94ffd9d6a25 100644 --- a/acceptance/bundle/migrate/auto/registered-model-rename/script +++ b/acceptance/bundle/migrate/auto/registered-model-rename/script @@ -10,12 +10,10 @@ title "Replay the prior terraform deployment (captured from a real terraform run trace replay_tfstate.py terraform-requests.json terraform.tfstate rm -f "$OUT_REQUESTS" -title "Rename the model (an immutable id field) and deploy on direct" -# name is a provided id field on registered models, so a rename recreates. reconcileIDFields -# seeds the deployed name into the converted state, so the migration's plan sees a real -# recreate. Unlike schemas/volumes, registered models are not in the destructive-approval -# group (a model recreate loses no data), so the deploy is approved without --auto-approve: -# the migration commits and the recreate applies on direct. +title "Rename the model (an updatable id field) and deploy on direct" +# name is an updatable id field: the model is renamed in place (UpdateWithID), keeping its +# versions. reconcileIDFields seeds the deployed name into the converted state, so the +# migration's plan sees a rename rather than snapshotting the new name as already applied. update_file.py databricks.yml "themodel-$UNIQUE_NAME" "themodel-$UNIQUE_NAME-renamed" trace DATABRICKS_BUNDLE_ENGINE=direct $CLI bundle deploy trace print_migration_telemetry diff --git a/acceptance/bundle/migrate/auto/volume-type-change/script b/acceptance/bundle/migrate/auto/volume-type-change/script index ea5ebdb1499..922c77c7b0c 100644 --- a/acceptance/bundle/migrate/auto/volume-type-change/script +++ b/acceptance/bundle/migrate/auto/volume-type-change/script @@ -2,7 +2,7 @@ export DATABRICKS_BUNDLE_ENGINE= # Testserver-only: changing volume_type to EXTERNAL needs a storage_location backed by a # registered external location on a real workspace, so this is not Cloud-enabled. The -# recreate class it covers is verified on cloud by schema-catalog-change / registered-model-rename. +# recreate class it covers is verified on cloud by schema-catalog-change / registered-model-catalog-change. # volume_type is both a provided id field and a recreate_on_changes field. reconcileIDFields # records the deployed value, so the migration's plan sees a real change and recreates. Volumes diff --git a/acceptance/bundle/resources/registered_models/basic/output.txt b/acceptance/bundle/resources/registered_models/basic/output.txt index 0cfe7b53077..a5bb3d2f6ca 100644 --- a/acceptance/bundle/resources/registered_models/basic/output.txt +++ b/acceptance/bundle/resources/registered_models/basic/output.txt @@ -58,17 +58,17 @@ Resources: 0 created, 1 changed, 0 deleted, 0 unchanged "schema_name": "default" } -=== update the name, this should recreate +=== update the name, this renames the model in place >>> [CLI] bundle plan -recreate registered_models.my_registered_model +update registered_models.my_registered_model -Plan: 1 to add, 0 to change, 1 to delete, 0 unchanged +Plan: 0 to add, 1 to change, 0 to delete, 0 unchanged >>> [CLI] bundle deploy Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/deploy-registered-models-basic-[UNIQUE_NAME]/default/files... -Recreated registered_models.my_registered_model +Updated registered_models.my_registered_model Files: 1 uploaded, 0 deleted -Resources: 1 created, 0 changed, 1 deleted, 0 unchanged +Resources: 0 created, 1 changed, 0 deleted, 0 unchanged >>> [CLI] registered-models get main.default.my-registered-model-updated-[UNIQUE_NAME] { diff --git a/acceptance/bundle/resources/registered_models/basic/script b/acceptance/bundle/resources/registered_models/basic/script index 61dff74182c..91b57c1e80a 100644 --- a/acceptance/bundle/resources/registered_models/basic/script +++ b/acceptance/bundle/resources/registered_models/basic/script @@ -36,7 +36,7 @@ deploy_registered_model export NAME="my-registered-model-updated-$UNIQUE_NAME" envsubst < databricks.yml.tmpl > databricks.yml -title "update the name, this should recreate" +title "update the name, this renames the model in place" deploy_registered_model title "update the catalog name, this should recreate" diff --git a/acceptance/bundle/resources/registered_models/rename/databricks.yml.tmpl b/acceptance/bundle/resources/registered_models/rename/databricks.yml.tmpl new file mode 100644 index 00000000000..957167b6ad5 --- /dev/null +++ b/acceptance/bundle/resources/registered_models/rename/databricks.yml.tmpl @@ -0,0 +1,16 @@ +bundle: + name: test-bundle-$UNIQUE_NAME + +resources: + catalogs: + catalog1: + name: test_catalog_$UNIQUE_NAME + schemas: + schema1: + catalog_name: ${resources.catalogs.catalog1.name} + name: myschema + registered_models: + model1: + catalog_name: ${resources.catalogs.catalog1.name} + schema_name: ${resources.schemas.schema1.name} + name: mymodel diff --git a/acceptance/bundle/resources/registered_models/rename/out.test.toml b/acceptance/bundle/resources/registered_models/rename/out.test.toml new file mode 100644 index 00000000000..0f167effef1 --- /dev/null +++ b/acceptance/bundle/resources/registered_models/rename/out.test.toml @@ -0,0 +1,2 @@ +Cloud = true +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/resources/registered_models/rename/output.txt b/acceptance/bundle/resources/registered_models/rename/output.txt new file mode 100644 index 00000000000..f5d604df265 --- /dev/null +++ b/acceptance/bundle/resources/registered_models/rename/output.txt @@ -0,0 +1,467 @@ + +=== Deploy a catalog with a schema and a registered model in it +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... +Created catalogs.catalog1 +Created registered_models.model1 +Created schemas.schema1 +Files: 2 uploaded, 0 deleted +Resources: 3 created, 0 changed, 0 deleted, 0 unchanged + +>>> print_requests.py //unity-catalog +{ + "method": "POST", + "path": "/api/2.1/unity-catalog/catalogs", + "body": { + "name": "test_catalog_[UNIQUE_NAME]" + } +} +{ + "method": "POST", + "path": "/api/2.1/unity-catalog/schemas", + "body": { + "catalog_name": "test_catalog_[UNIQUE_NAME]", + "name": "myschema" + } +} +{ + "method": "POST", + "path": "/api/2.1/unity-catalog/models", + "body": { + "catalog_name": "test_catalog_[UNIQUE_NAME]", + "name": "mymodel", + "schema_name": "myschema" + } +} + +=== Rename the model: it is renamed in place, keeping its versions +>>> update_file.py databricks.yml name: mymodel name: mymodel2 + +>>> [CLI] bundle plan --output json +{ + "resources.catalogs.catalog1": { + "action": "skip", + "remote_state": { + "catalog_type": "MANAGED_CATALOG", + "created_by": "[USERNAME]", + "full_name": "test_catalog_[UNIQUE_NAME]", + "name": "test_catalog_[UNIQUE_NAME]", + "owner": "[USERNAME]" + } + }, + "resources.registered_models.model1": { + "depends_on": [ + { + "node": "resources.catalogs.catalog1", + "label": "${resources.catalogs.catalog1.name}" + }, + { + "node": "resources.schemas.schema1", + "label": "${resources.schemas.schema1.name}" + } + ], + "action": "update_id", + "new_state": { + "value": { + "catalog_name": "test_catalog_[UNIQUE_NAME]", + "name": "mymodel2", + "schema_name": "myschema" + } + }, + "remote_state": { + "catalog_name": "test_catalog_[UNIQUE_NAME]", + "created_by": "[USERNAME]", + "full_name": "test_catalog_[UNIQUE_NAME].myschema.mymodel", + "name": "mymodel", + "owner": "[USERNAME]", + "schema_name": "myschema" + }, + "changes": { + "created_by": { + "action": "skip", + "reason": "output_only", + "remote": "[USERNAME]" + }, + "full_name": { + "action": "skip", + "reason": "backend_default", + "remote": "test_catalog_[UNIQUE_NAME].myschema.mymodel" + }, + "name": { + "action": "update_id", + "reason": "id_changes", + "old": "mymodel", + "new": "mymodel2", + "remote": "mymodel" + }, + "owner": { + "action": "skip", + "reason": "backend_default", + "remote": "[USERNAME]" + } + } + }, + "resources.schemas.schema1": { + "depends_on": [ + { + "node": "resources.catalogs.catalog1", + "label": "${resources.catalogs.catalog1.name}" + } + ], + "action": "skip", + "remote_state": { + "catalog_name": "test_catalog_[UNIQUE_NAME]", + "catalog_type": "MANAGED_CATALOG", + "created_by": "[USERNAME]", + "full_name": "test_catalog_[UNIQUE_NAME].myschema", + "name": "myschema", + "owner": "[USERNAME]", + "schema_id": "[UUID]" + } + } +} + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... +Updated registered_models.model1 +Files: 1 uploaded, 0 deleted +Resources: 0 created, 1 changed, 0 deleted, 2 unchanged + +>>> print_requests.py //unity-catalog +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/models/test_catalog_[UNIQUE_NAME].myschema.mymodel", + "body": { + "catalog_name": "test_catalog_[UNIQUE_NAME]", + "comment": "", + "new_name": "mymodel2", + "schema_name": "myschema" + } +} + +>>> [CLI] registered-models get test_catalog_[UNIQUE_NAME].myschema.mymodel2 +{ + "full_name": "test_catalog_[UNIQUE_NAME].myschema.mymodel2", + "created_at": [MODEL_CREATED_AT] +} + +=== Rename the schema: UC moves the model with it, so the model is re-keyed without a request +>>> update_file.py databricks.yml name: myschema name: myschema2 + +>>> [CLI] bundle plan --output json +{ + "resources.catalogs.catalog1": { + "action": "skip", + "remote_state": { + "catalog_type": "MANAGED_CATALOG", + "created_by": "[USERNAME]", + "full_name": "test_catalog_[UNIQUE_NAME]", + "name": "test_catalog_[UNIQUE_NAME]", + "owner": "[USERNAME]" + } + }, + "resources.registered_models.model1": { + "depends_on": [ + { + "node": "resources.catalogs.catalog1", + "label": "${resources.catalogs.catalog1.name}" + }, + { + "node": "resources.schemas.schema1", + "label": "${resources.schemas.schema1.name}" + } + ], + "action": "update_id", + "new_state": { + "value": { + "catalog_name": "test_catalog_[UNIQUE_NAME]", + "name": "mymodel2", + "schema_name": "myschema2" + } + }, + "remote_state": { + "catalog_name": "test_catalog_[UNIQUE_NAME]", + "comment": "", + "created_by": "[USERNAME]", + "full_name": "test_catalog_[UNIQUE_NAME].myschema.mymodel2", + "name": "mymodel2", + "owner": "[USERNAME]", + "schema_name": "myschema" + }, + "changes": { + "comment": { + "action": "skip", + "reason": "empty", + "remote": "" + }, + "created_by": { + "action": "skip", + "reason": "output_only", + "remote": "[USERNAME]" + }, + "full_name": { + "action": "skip", + "reason": "backend_default", + "remote": "test_catalog_[UNIQUE_NAME].myschema.mymodel2" + }, + "owner": { + "action": "skip", + "reason": "backend_default", + "remote": "[USERNAME]" + }, + "schema_name": { + "action": "update_id", + "reason": "parent_renamed", + "old": "myschema", + "new": "myschema2", + "remote": "myschema" + } + } + }, + "resources.schemas.schema1": { + "depends_on": [ + { + "node": "resources.catalogs.catalog1", + "label": "${resources.catalogs.catalog1.name}" + } + ], + "action": "update_id", + "new_state": { + "value": { + "catalog_name": "test_catalog_[UNIQUE_NAME]", + "name": "myschema2" + } + }, + "remote_state": { + "catalog_name": "test_catalog_[UNIQUE_NAME]", + "catalog_type": "MANAGED_CATALOG", + "created_by": "[USERNAME]", + "full_name": "test_catalog_[UNIQUE_NAME].myschema", + "name": "myschema", + "owner": "[USERNAME]", + "schema_id": "[UUID]" + }, + "changes": { + "name": { + "action": "update_id", + "reason": "id_changes", + "old": "myschema", + "new": "myschema2", + "remote": "myschema" + } + } + } +} + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... +Updated registered_models.model1 +Updated schemas.schema1 +Files: 1 uploaded, 0 deleted +Resources: 0 created, 2 changed, 0 deleted, 1 unchanged + +>>> print_requests.py //unity-catalog +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/schemas/test_catalog_[UNIQUE_NAME].myschema", + "body": { + "comment": "", + "custom_max_retention_hours": 0, + "new_name": "myschema2" + } +} + +>>> [CLI] registered-models get test_catalog_[UNIQUE_NAME].myschema2.mymodel2 +{ + "full_name": "test_catalog_[UNIQUE_NAME].myschema2.mymodel2", + "created_at": [MODEL_CREATED_AT] +} + +=== Rename the catalog: the model is re-keyed again +>>> update_file.py databricks.yml test_catalog_ test_catalog_renamed_ + +>>> [CLI] bundle plan --output json +{ + "resources.catalogs.catalog1": { + "action": "update_id", + "new_state": { + "value": { + "name": "test_catalog_renamed_[UNIQUE_NAME]" + } + }, + "remote_state": { + "catalog_type": "MANAGED_CATALOG", + "created_by": "[USERNAME]", + "full_name": "test_catalog_[UNIQUE_NAME]", + "name": "test_catalog_[UNIQUE_NAME]", + "owner": "[USERNAME]" + }, + "changes": { + "name": { + "action": "update_id", + "reason": "id_changes", + "old": "test_catalog_[UNIQUE_NAME]", + "new": "test_catalog_renamed_[UNIQUE_NAME]", + "remote": "test_catalog_[UNIQUE_NAME]" + } + } + }, + "resources.registered_models.model1": { + "depends_on": [ + { + "node": "resources.catalogs.catalog1", + "label": "${resources.catalogs.catalog1.name}" + }, + { + "node": "resources.schemas.schema1", + "label": "${resources.schemas.schema1.name}" + } + ], + "action": "update_id", + "new_state": { + "value": { + "catalog_name": "test_catalog_renamed_[UNIQUE_NAME]", + "name": "mymodel2", + "schema_name": "myschema2" + } + }, + "remote_state": { + "catalog_name": "test_catalog_[UNIQUE_NAME]", + "comment": "", + "created_by": "[USERNAME]", + "full_name": "test_catalog_[UNIQUE_NAME].myschema2.mymodel2", + "name": "mymodel2", + "owner": "[USERNAME]", + "schema_name": "myschema2" + }, + "changes": { + "catalog_name": { + "action": "update_id", + "reason": "parent_renamed", + "old": "test_catalog_[UNIQUE_NAME]", + "new": "test_catalog_renamed_[UNIQUE_NAME]", + "remote": "test_catalog_[UNIQUE_NAME]" + }, + "comment": { + "action": "skip", + "reason": "empty", + "remote": "" + }, + "created_by": { + "action": "skip", + "reason": "output_only", + "remote": "[USERNAME]" + }, + "full_name": { + "action": "skip", + "reason": "backend_default", + "remote": "test_catalog_[UNIQUE_NAME].myschema2.mymodel2" + }, + "owner": { + "action": "skip", + "reason": "backend_default", + "remote": "[USERNAME]" + } + } + }, + "resources.schemas.schema1": { + "depends_on": [ + { + "node": "resources.catalogs.catalog1", + "label": "${resources.catalogs.catalog1.name}" + } + ], + "action": "update_id", + "new_state": { + "value": { + "catalog_name": "test_catalog_renamed_[UNIQUE_NAME]", + "name": "myschema2" + } + }, + "remote_state": { + "catalog_name": "test_catalog_[UNIQUE_NAME]", + "catalog_type": "MANAGED_CATALOG", + "comment": "", + "created_by": "[USERNAME]", + "custom_max_retention_hours": 0, + "full_name": "test_catalog_[UNIQUE_NAME].myschema2", + "name": "myschema2", + "owner": "[USERNAME]", + "schema_id": "[UUID]" + }, + "changes": { + "catalog_name": { + "action": "update_id", + "reason": "parent_renamed", + "old": "test_catalog_[UNIQUE_NAME]", + "new": "test_catalog_renamed_[UNIQUE_NAME]", + "remote": "test_catalog_[UNIQUE_NAME]" + }, + "comment": { + "action": "skip", + "reason": "empty", + "remote": "" + }, + "custom_max_retention_hours": { + "action": "skip", + "reason": "empty", + "remote": 0 + } + } + } +} + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... +Updated catalogs.catalog1 +Updated registered_models.model1 +Updated schemas.schema1 +Files: 1 uploaded, 0 deleted +Resources: 0 created, 3 changed, 0 deleted, 0 unchanged + +>>> print_requests.py //unity-catalog +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/catalogs/test_catalog_[UNIQUE_NAME]", + "body": { + "comment": "", + "custom_max_retention_hours": 0, + "new_name": "test_catalog_renamed_[UNIQUE_NAME]" + } +} + +>>> [CLI] registered-models get test_catalog_renamed_[UNIQUE_NAME].myschema2.mymodel2 +{ + "full_name": "test_catalog_renamed_[UNIQUE_NAME].myschema2.mymodel2", + "created_at": [MODEL_CREATED_AT] +} + +=== The state tracks the new paths and the plan is a no-op +>>> [CLI] bundle summary --output json +{ + "catalogs": { + "catalog1": "test_catalog_renamed_[UNIQUE_NAME]" + }, + "registered_models": { + "model1": "test_catalog_renamed_[UNIQUE_NAME].myschema2.mymodel2" + }, + "schemas": { + "schema1": "test_catalog_renamed_[UNIQUE_NAME].myschema2" + } +} + +>>> [CLI] bundle plan +Plan: 0 to add, 0 to change, 0 to delete, 3 unchanged + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.catalogs.catalog1 + delete resources.registered_models.model1 + delete resources.schemas.schema1 + +This action will result in the deletion of the following UC schemas. Any underlying data may be lost: + delete resources.schemas.schema1 + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default + +Destroy: 3 deleted diff --git a/acceptance/bundle/resources/registered_models/rename/script b/acceptance/bundle/resources/registered_models/rename/script new file mode 100644 index 00000000000..09f2bf95ca3 --- /dev/null +++ b/acceptance/bundle/resources/registered_models/rename/script @@ -0,0 +1,44 @@ +envsubst < databricks.yml.tmpl > databricks.yml + +cleanup() { + trace $CLI bundle destroy --auto-approve + + # Consume the destroy's requests so none are left recorded. + print_requests.py //unity-catalog &> LOG.destroy_requests +} +trap cleanup EXIT + +# Drop the remote_state fields that differ between clouds or from the test server. +plan() { + trace $CLI bundle plan --output json | jq .plan | normalize_uc_payload.py created_at updated_at updated_by metastore_id browse_only effective_predictive_optimization_flag enable_predictive_optimization isolation_mode securable_type storage_location +} + +title "Deploy a catalog with a schema and a registered model in it" +trace $CLI bundle deploy +trace print_requests.py //unity-catalog +add_repl "$($CLI registered-models get "test_catalog_${UNIQUE_NAME}.myschema.mymodel" | jq -r .created_at)" MODEL_CREATED_AT + +title "Rename the model: it is renamed in place, keeping its versions" +trace update_file.py databricks.yml "name: mymodel" "name: mymodel2" +plan +trace $CLI bundle deploy +trace print_requests.py //unity-catalog +trace $CLI registered-models get "test_catalog_${UNIQUE_NAME}.myschema.mymodel2" | jq '{full_name, created_at}' + +title "Rename the schema: UC moves the model with it, so the model is re-keyed without a request" +trace update_file.py databricks.yml "name: myschema" "name: myschema2" +plan +trace $CLI bundle deploy +trace print_requests.py //unity-catalog +trace $CLI registered-models get "test_catalog_${UNIQUE_NAME}.myschema2.mymodel2" | jq '{full_name, created_at}' + +title "Rename the catalog: the model is re-keyed again" +trace update_file.py databricks.yml test_catalog_ test_catalog_renamed_ +plan +trace $CLI bundle deploy +trace print_requests.py //unity-catalog +trace $CLI registered-models get "test_catalog_renamed_${UNIQUE_NAME}.myschema2.mymodel2" | jq '{full_name, created_at}' + +title "The state tracks the new paths and the plan is a no-op" +trace $CLI bundle summary --output json | jq '.resources | map_values(map_values(.id))' +trace $CLI bundle plan diff --git a/acceptance/bundle/resources/registered_models/rename/test.toml b/acceptance/bundle/resources/registered_models/rename/test.toml new file mode 100644 index 00000000000..4272a80cf61 --- /dev/null +++ b/acceptance/bundle/resources/registered_models/rename/test.toml @@ -0,0 +1,7 @@ +Cloud = true + +Ignore = [ + ".databricks", + ".gitignore", + "databricks.yml", +] diff --git a/bundle/direct/bundle_plan_test.go b/bundle/direct/bundle_plan_test.go index f50de986918..28495c84cdd 100644 --- a/bundle/direct/bundle_plan_test.go +++ b/bundle/direct/bundle_plan_test.go @@ -301,8 +301,8 @@ func TestFollowParentRenames(t *testing.T) { }, { name: "resource without DoUpdateWithID is recreated", - resource: "registered_models", - field: "schema_name", + resource: "quality_monitors", + field: "table_name", old: "s_old", expectedAction: deployplan.Recreate, }, diff --git a/bundle/direct/dresources/configs/registered_models.yml b/bundle/direct/dresources/configs/registered_models.yml index 03bcbcc6079..a0c5345b75a 100644 --- a/bundle/direct/dresources/configs/registered_models.yml +++ b/bundle/direct/dresources/configs/registered_models.yml @@ -20,14 +20,13 @@ ignore_remote_changes: - field: aliases[*].model_name reason: output_only provided_id_fields: - # The name can technically be updated without recreate. We recreate for now though - # to match TF implementation. - - field: name - reason: terraform_compat - field: catalog_name reason: id_field - field: schema_name reason: id_field +updatable_id_fields: + - field: name + reason: id_changes recreate_on_changes: - field: storage_location reason: immutable diff --git a/bundle/direct/dresources/registered_model.go b/bundle/direct/dresources/registered_model.go index 9dd97965bce..b9bb68de2ac 100644 --- a/bundle/direct/dresources/registered_model.go +++ b/bundle/direct/dresources/registered_model.go @@ -91,9 +91,14 @@ func (r *ResourceRegisteredModel) WaitAfterCreate(ctx context.Context, id string // See schemaForceSend. Verified against a real workspace: {"comment": ""} clears it. var registeredModelForceSend = []string{"Comment"} -func (r *ResourceRegisteredModel) DoUpdate(ctx context.Context, id string, config *catalog.CreateRegisteredModelRequest, entry *PlanEntry) (*catalog.RegisteredModelInfo, error) { - updateRequest := catalog.UpdateRegisteredModelRequest{ - FullName: id, +func makeUpdateRegisteredModel(fullName, newName string, config *catalog.CreateRegisteredModelRequest) catalog.UpdateRegisteredModelRequest { + // UC rejects an update that sets both name and new_name. + name := config.Name + if newName != "" { + name = "" + } + return catalog.UpdateRegisteredModelRequest{ + FullName: fullName, Comment: config.Comment, ForceSendFields: utils.FilterFields[catalog.UpdateRegisteredModelRequest](append(slices.Clone(registeredModelForceSend), config.ForceSendFields...), "Owner", "NewName"), @@ -102,9 +107,8 @@ func (r *ResourceRegisteredModel) DoUpdate(ctx context.Context, id string, confi // change to it is silently dropped rather than applied. Owner: "", - // Name updates are not supported yet without recreating. Can be added as a follow-up. - // Note: TF also does not support changing name without a recreate so the current behavior matches TF. - NewName: "", + // Set by DoUpdateWithID when the name changes. + NewName: newName, // The update API ignores aliases; syncAliases applies them through the alias API. Aliases: nil, @@ -117,27 +121,59 @@ func (r *ResourceRegisteredModel) DoUpdate(ctx context.Context, id string, confi UpdatedBy: config.UpdatedBy, SchemaName: config.SchemaName, StorageLocation: config.StorageLocation, - Name: config.Name, + Name: name, CatalogName: config.CatalogName, } +} - response, err := r.client.RegisteredModels.Update(ctx, updateRequest) +func (r *ResourceRegisteredModel) DoUpdate(ctx context.Context, id string, config *catalog.CreateRegisteredModelRequest, entry *PlanEntry) (*catalog.RegisteredModelInfo, error) { + response, err := r.client.RegisteredModels.Update(ctx, makeUpdateRegisteredModel(id, "", config)) if err != nil { return nil, err } + return response, r.syncChangedAliases(ctx, id, config, entry) +} - if entry.Changes.HasChange(registeredModelAliasesPath) { - remote, ok := entry.RemoteState.(*catalog.RegisteredModelInfo) - if !ok { - return nil, fmt.Errorf("internal error: unexpected remote state type %T", entry.RemoteState) - } - err = r.syncAliases(ctx, id, config.Aliases, remote.Aliases) - if err != nil { - return nil, err - } +// DoUpdateWithID renames the model in place when the name changes, keeping its versions; the ID is +// the full name, so it changes too. +func (r *ResourceRegisteredModel) DoUpdateWithID(ctx context.Context, id string, config *catalog.CreateRegisteredModelRequest, entry *PlanEntry) (string, *catalog.RegisteredModelInfo, error) { + nameFromID, err := getNameFromID(id) + if err != nil { + return "", nil, err } - return response, nil + // The catalog and schema come from config: a renamed parent moves the model with it, leaving id + // stale (see followParentRenames). + parent := config.CatalogName + "." + config.SchemaName + "." + newID := parent + config.Name + + if entry.Changes.OnlyParentRenamed() { + // UC already moved the model with its parent; only the ID changes. + return newID, nil, nil + } + + newName := "" + if config.Name != nameFromID { + newName = config.Name + } + + response, err := r.client.RegisteredModels.Update(ctx, makeUpdateRegisteredModel(parent+nameFromID, newName, config)) + if err != nil { + return "", nil, err + } + return newID, response, r.syncChangedAliases(ctx, newID, config, entry) +} + +// syncChangedAliases applies the configured aliases when the plan changes them. +func (r *ResourceRegisteredModel) syncChangedAliases(ctx context.Context, fullName string, config *catalog.CreateRegisteredModelRequest, entry *PlanEntry) error { + if !entry.Changes.HasChange(registeredModelAliasesPath) { + return nil + } + remote, ok := entry.RemoteState.(*catalog.RegisteredModelInfo) + if !ok { + return fmt.Errorf("internal error: unexpected remote state type %T", entry.RemoteState) + } + return r.syncAliases(ctx, fullName, config.Aliases, remote.Aliases) } // syncAliases makes the model's aliases match desired, given the ones it has remotely. The alias API diff --git a/libs/testserver/catalogs.go b/libs/testserver/catalogs.go index ff62b2e6959..27ee4f53102 100644 --- a/libs/testserver/catalogs.go +++ b/libs/testserver/catalogs.go @@ -133,7 +133,7 @@ func (s *FakeWorkspace) CatalogsUpdate(req Request, name string) Response { } } -// moveCatalogChildren mirrors UC, where the schemas and volumes of a renamed catalog move with it. +// moveCatalogChildren mirrors UC, where the schemas, volumes and registered models of a renamed catalog move with it. func (s *FakeWorkspace) moveCatalogChildren(oldCatalogName, newCatalogName string) { for key, schema := range s.Schemas { if schema.CatalogName != oldCatalogName { @@ -153,9 +153,16 @@ func (s *FakeWorkspace) moveCatalogChildren(oldCatalogName, newCatalogName strin volume.FullName = newCatalogName + "." + volume.SchemaName + "." + volume.Name s.Volumes[volume.FullName] = volume } + for key, model := range s.RegisteredModels { + if model.CatalogName != oldCatalogName { + continue + } + model.CatalogName = newCatalogName + s.moveRegisteredModel(key, model) + } } -// CatalogsDelete mirrors UC, where deleting a catalog with force deletes its schemas and volumes too. +// CatalogsDelete mirrors UC, where deleting a catalog with force deletes its schemas, volumes and registered models too. func (s *FakeWorkspace) CatalogsDelete(req Request, name string) Response { defer s.LockUnlock()() @@ -174,6 +181,12 @@ func (s *FakeWorkspace) CatalogsDelete(req Request, name string) Response { delete(s.Volumes, key) } } + for key, model := range s.RegisteredModels { + if model.CatalogName == name { + delete(s.RegisteredModels, key) + delete(s.RegisteredModelVersions, key) + } + } } return Response{} } diff --git a/libs/testserver/registered_models.go b/libs/testserver/registered_models.go index 7e178e4b29b..2dea8ecc864 100644 --- a/libs/testserver/registered_models.go +++ b/libs/testserver/registered_models.go @@ -77,25 +77,46 @@ func (s *FakeWorkspace) RegisteredModelsUpdate(req Request, fullName string) Res } } + if updateRequest.NewName != "" && updateRequest.Name != "" { + return Response{ + StatusCode: http.StatusBadRequest, + Body: map[string]string{ + "error_code": "INVALID_PARAMETER_VALUE", + "message": "UpdateRegisteredModel Both new_name and name cannot be specified.", + }, + } + } + // The update API ignores aliases; they are set through the alias endpoints. delete(fields, "aliases") applyUpdatedFields(&existing, updateRequest, fields) if updateRequest.NewName != "" { existing.Name = updateRequest.NewName - - // Delete the old entry and set full name to the new name - delete(s.RegisteredModels, fullName) - fullName = existing.CatalogName + "." + existing.SchemaName + "." + updateRequest.NewName } existing.UpdatedAt = nowMilli() - s.RegisteredModels[fullName] = existing + existing = s.moveRegisteredModel(fullName, existing) return Response{ Body: existing, } } +// moveRegisteredModel stores the model under the full name its catalog, schema and name now give it, +// keeping its versions, as UC does when the model or its parent is renamed. +func (s *FakeWorkspace) moveRegisteredModel(oldFullName string, model catalog.RegisteredModelInfo) catalog.RegisteredModelInfo { + delete(s.RegisteredModels, oldFullName) + model.FullName = model.CatalogName + "." + model.SchemaName + "." + model.Name + s.RegisteredModels[model.FullName] = model + + versions, ok := s.RegisteredModelVersions[oldFullName] + if ok { + delete(s.RegisteredModelVersions, oldFullName) + s.RegisteredModelVersions[model.FullName] = versions + } + return model +} + // RegisteredModelsGet mirrors UC, which lists a model's aliases only when include_aliases is set. func (s *FakeWorkspace) RegisteredModelsGet(req Request, fullName string) Response { defer s.LockUnlock()() diff --git a/libs/testserver/schemas.go b/libs/testserver/schemas.go index 8b3403a0204..0f77cbe5640 100644 --- a/libs/testserver/schemas.go +++ b/libs/testserver/schemas.go @@ -103,7 +103,7 @@ func (s *FakeWorkspace) SchemasUpdate(req Request, name string) Response { // UC normalizes schema names to lowercase. existing.Name = strings.ToLower(schemaUpdate.NewName) existing.FullName = existing.CatalogName + "." + existing.Name - s.moveSchemaVolumes(existing.CatalogName, oldName, existing.Name) + s.moveSchemaChildren(existing.CatalogName, oldName, existing.Name) } existing.UpdatedAt = nowMilli() @@ -116,8 +116,8 @@ func (s *FakeWorkspace) SchemasUpdate(req Request, name string) Response { } } -// moveSchemaVolumes mirrors UC, where the volumes of a renamed schema move with it. -func (s *FakeWorkspace) moveSchemaVolumes(catalogName, oldSchemaName, newSchemaName string) { +// moveSchemaChildren mirrors UC, where the volumes and registered models of a renamed schema move with it. +func (s *FakeWorkspace) moveSchemaChildren(catalogName, oldSchemaName, newSchemaName string) { for key, volume := range s.Volumes { if volume.CatalogName != catalogName || volume.SchemaName != oldSchemaName { continue @@ -127,9 +127,16 @@ func (s *FakeWorkspace) moveSchemaVolumes(catalogName, oldSchemaName, newSchemaN volume.FullName = catalogName + "." + newSchemaName + "." + volume.Name s.Volumes[volume.FullName] = volume } + for key, model := range s.RegisteredModels { + if model.CatalogName != catalogName || model.SchemaName != oldSchemaName { + continue + } + model.SchemaName = newSchemaName + s.moveRegisteredModel(key, model) + } } -// SchemasDelete mirrors UC, where deleting a schema with force deletes the volumes in it too. +// SchemasDelete mirrors UC, where deleting a schema with force deletes the volumes and registered models in it too. func (s *FakeWorkspace) SchemasDelete(req Request, fullName string) Response { defer s.LockUnlock()() @@ -144,6 +151,12 @@ func (s *FakeWorkspace) SchemasDelete(req Request, fullName string) Response { delete(s.Volumes, key) } } + for key, model := range s.RegisteredModels { + if model.CatalogName == schema.CatalogName && model.SchemaName == schema.Name { + delete(s.RegisteredModels, key) + delete(s.RegisteredModelVersions, key) + } + } } return Response{} }