diff --git a/Makefile b/Makefile index 5714483a36..573708d2b4 100644 --- a/Makefile +++ b/Makefile @@ -96,9 +96,14 @@ test-slow-acc: # Updates acceptance test output (local tests) .PHONY: test-update -test-update: +test-update: test-update-golden -go test ./acceptance -run '^TestAccept$$' -update -timeout=${LOCAL_TIMEOUT} +# Updates golden files test output (other than acceptance tests) +.PHONY: test-update-golden +test-update-golden: + ./tools/update_golden.py + # Updates acceptance test output for template tests only .PHONY: test-update-templates test-update-templates: diff --git a/bundle/direct/dresources/catalog.go b/bundle/direct/dresources/catalog.go index bb1fb82490..649a56230d 100644 --- a/bundle/direct/dresources/catalog.go +++ b/bundle/direct/dresources/catalog.go @@ -4,7 +4,6 @@ import ( "context" "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/libs/utils" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/catalog" ) @@ -21,18 +20,11 @@ func (*ResourceCatalog) PrepareState(input *resources.Catalog) *catalog.CreateCa return &input.CreateCatalog } +// catalogRemapCopy maps CatalogInfo (remote GET response) to CreateCatalog (local state). +var catalogRemapCopy = newCopy[catalog.CatalogInfo, catalog.CreateCatalog]() + func (*ResourceCatalog) RemapState(info *catalog.CatalogInfo) *catalog.CreateCatalog { - return &catalog.CreateCatalog{ - Comment: info.Comment, - ConnectionName: info.ConnectionName, - Name: info.Name, - Options: info.Options, - Properties: info.Properties, - ProviderName: info.ProviderName, - ShareName: info.ShareName, - StorageRoot: info.StorageRoot, - ForceSendFields: utils.FilterFields[catalog.CreateCatalog](info.ForceSendFields), - } + return catalogRemapCopy.Do(info) } func (r *ResourceCatalog) DoRead(ctx context.Context, id string) (*catalog.CatalogInfo, error) { @@ -47,21 +39,15 @@ func (r *ResourceCatalog) DoCreate(ctx context.Context, config *catalog.CreateCa return response.Name, response, nil } +// catalogUpdateCopy maps CreateCatalog (local state) to UpdateCatalog (API request). +var catalogUpdateCopy = newCopy[catalog.CreateCatalog, catalog.UpdateCatalog]() + // DoUpdate updates the catalog in place and returns remote state. func (r *ResourceCatalog) DoUpdate(ctx context.Context, id string, config *catalog.CreateCatalog, _ Changes) (*catalog.CatalogInfo, error) { - updateRequest := catalog.UpdateCatalog{ - Comment: config.Comment, - EnablePredictiveOptimization: "", // Not supported by DABs - IsolationMode: "", // Not supported by DABs - Name: id, - NewName: "", // Only set if name actually changes (see DoUpdateWithID) - Options: config.Options, - Owner: "", // Not supported by DABs - Properties: config.Properties, - ForceSendFields: utils.FilterFields[catalog.UpdateCatalog](config.ForceSendFields, "EnablePredictiveOptimization", "IsolationMode", "Owner"), - } + updateRequest := catalogUpdateCopy.Do(config) + updateRequest.Name = id - response, err := r.client.Catalogs.Update(ctx, updateRequest) + response, err := r.client.Catalogs.Update(ctx, *updateRequest) if err != nil { return nil, err } @@ -71,23 +57,14 @@ func (r *ResourceCatalog) DoUpdate(ctx context.Context, id string, config *catal // DoUpdateWithID updates the catalog and returns the new ID if the name changes. func (r *ResourceCatalog) DoUpdateWithID(ctx context.Context, id string, config *catalog.CreateCatalog) (string, *catalog.CatalogInfo, error) { - updateRequest := catalog.UpdateCatalog{ - Comment: config.Comment, - EnablePredictiveOptimization: "", // Not supported by DABs - IsolationMode: "", // Not supported by DABs - Name: id, - NewName: "", // Initialized below if needed - Options: config.Options, - Owner: "", // Not supported by DABs - Properties: config.Properties, - ForceSendFields: utils.FilterFields[catalog.UpdateCatalog](config.ForceSendFields, "EnablePredictiveOptimization", "IsolationMode", "Owner"), - } + updateRequest := catalogUpdateCopy.Do(config) + updateRequest.Name = id if config.Name != id { updateRequest.NewName = config.Name } - response, err := r.client.Catalogs.Update(ctx, updateRequest) + response, err := r.client.Catalogs.Update(ctx, *updateRequest) if err != nil { return "", nil, err } diff --git a/bundle/direct/dresources/cluster.go b/bundle/direct/dresources/cluster.go index a8f78d12f9..f9b2a80015 100644 --- a/bundle/direct/dresources/cluster.go +++ b/bundle/direct/dresources/cluster.go @@ -30,44 +30,11 @@ func (r *ResourceCluster) PrepareState(input *resources.Cluster) *compute.Cluste return &input.ClusterSpec } +// clusterRemapCopy maps ClusterDetails (remote GET response) to ClusterSpec (local state). +var clusterRemapCopy = newCopy[compute.ClusterDetails, compute.ClusterSpec]() + func (r *ResourceCluster) RemapState(input *compute.ClusterDetails) *compute.ClusterSpec { - spec := &compute.ClusterSpec{ - ApplyPolicyDefaultValues: false, - Autoscale: input.Autoscale, - AutoterminationMinutes: input.AutoterminationMinutes, - AwsAttributes: input.AwsAttributes, - AzureAttributes: input.AzureAttributes, - ClusterLogConf: input.ClusterLogConf, - ClusterName: input.ClusterName, - CustomTags: input.CustomTags, - DataSecurityMode: input.DataSecurityMode, - DockerImage: input.DockerImage, - DriverInstancePoolId: input.DriverInstancePoolId, - DriverNodeTypeId: input.DriverNodeTypeId, - DriverNodeTypeFlexibility: input.DriverNodeTypeFlexibility, - EnableElasticDisk: input.EnableElasticDisk, - EnableLocalDiskEncryption: input.EnableLocalDiskEncryption, - GcpAttributes: input.GcpAttributes, - InitScripts: input.InitScripts, - InstancePoolId: input.InstancePoolId, - IsSingleNode: input.IsSingleNode, - Kind: input.Kind, - NodeTypeId: input.NodeTypeId, - NumWorkers: input.NumWorkers, - PolicyId: input.PolicyId, - RemoteDiskThroughput: input.RemoteDiskThroughput, - RuntimeEngine: input.RuntimeEngine, - SingleUserName: input.SingleUserName, - SparkConf: input.SparkConf, - SparkEnvVars: input.SparkEnvVars, - SparkVersion: input.SparkVersion, - SshPublicKeys: input.SshPublicKeys, - TotalInitialRemoteDiskSize: input.TotalInitialRemoteDiskSize, - UseMlRuntime: input.UseMlRuntime, - WorkloadType: input.WorkloadType, - WorkerNodeTypeFlexibility: input.WorkerNodeTypeFlexibility, - ForceSendFields: utils.FilterFields[compute.ClusterSpec](input.ForceSendFields), - } + spec := clusterRemapCopy.Do(input) if input.Spec != nil { spec.ApplyPolicyDefaultValues = input.Spec.ApplyPolicyDefaultValues } @@ -78,20 +45,32 @@ func (r *ResourceCluster) DoRead(ctx context.Context, id string) (*compute.Clust return r.client.Clusters.GetByClusterId(ctx, id) } +// clusterCreateCopy maps ClusterSpec (local state) to CreateCluster (API request). +var clusterCreateCopy = newCopy[compute.ClusterSpec, compute.CreateCluster]() + func (r *ResourceCluster) DoCreate(ctx context.Context, config *compute.ClusterSpec) (string, *compute.ClusterDetails, error) { - wait, err := r.client.Clusters.Create(ctx, makeCreateCluster(config)) + create := clusterCreateCopy.Do(config) + forceNumWorkers(config, &create.ForceSendFields) + wait, err := r.client.Clusters.Create(ctx, *create) if err != nil { return "", nil, err } return wait.ClusterId, nil, nil } +// clusterEditCopy maps ClusterSpec (local state) to EditCluster (API request). +var clusterEditCopy = newCopy[compute.ClusterSpec, compute.EditCluster]() + func (r *ResourceCluster) DoUpdate(ctx context.Context, id string, config *compute.ClusterSpec, _ Changes) (*compute.ClusterDetails, error) { + edit := clusterEditCopy.Do(config) + edit.ClusterId = id + forceNumWorkers(config, &edit.ForceSendFields) + // Same retry as in TF provider logic // https://github.com/databricks/terraform-provider-databricks/blob/3eecd0f90cf99d7777e79a3d03c41f9b2aafb004/clusters/resource_cluster.go#L624 timeout := 15 * time.Minute _, err := retries.Poll(ctx, timeout, func() (*compute.WaitGetClusterRunning[struct{}], *retries.Err) { - wait, err := r.client.Clusters.Edit(ctx, makeEditCluster(id, config)) + wait, err := r.client.Clusters.Edit(ctx, *edit) if err == nil { return wait, nil } @@ -151,100 +130,10 @@ func (r *ResourceCluster) OverrideChangeDesc(ctx context.Context, p *structpath. return nil } -func makeCreateCluster(config *compute.ClusterSpec) compute.CreateCluster { - create := compute.CreateCluster{ - ApplyPolicyDefaultValues: config.ApplyPolicyDefaultValues, - Autoscale: config.Autoscale, - AutoterminationMinutes: config.AutoterminationMinutes, - AwsAttributes: config.AwsAttributes, - AzureAttributes: config.AzureAttributes, - ClusterLogConf: config.ClusterLogConf, - ClusterName: config.ClusterName, - CloneFrom: nil, // Not supported by DABs - CustomTags: config.CustomTags, - DataSecurityMode: config.DataSecurityMode, - DockerImage: config.DockerImage, - DriverInstancePoolId: config.DriverInstancePoolId, - DriverNodeTypeId: config.DriverNodeTypeId, - DriverNodeTypeFlexibility: config.DriverNodeTypeFlexibility, - EnableElasticDisk: config.EnableElasticDisk, - EnableLocalDiskEncryption: config.EnableLocalDiskEncryption, - GcpAttributes: config.GcpAttributes, - InitScripts: config.InitScripts, - InstancePoolId: config.InstancePoolId, - IsSingleNode: config.IsSingleNode, - Kind: config.Kind, - NodeTypeId: config.NodeTypeId, - NumWorkers: config.NumWorkers, - PolicyId: config.PolicyId, - RemoteDiskThroughput: config.RemoteDiskThroughput, - RuntimeEngine: config.RuntimeEngine, - SingleUserName: config.SingleUserName, - SparkConf: config.SparkConf, - SparkEnvVars: config.SparkEnvVars, - SparkVersion: config.SparkVersion, - SshPublicKeys: config.SshPublicKeys, - TotalInitialRemoteDiskSize: config.TotalInitialRemoteDiskSize, - UseMlRuntime: config.UseMlRuntime, - WorkloadType: config.WorkloadType, - WorkerNodeTypeFlexibility: config.WorkerNodeTypeFlexibility, - ForceSendFields: utils.FilterFields[compute.CreateCluster](config.ForceSendFields), - } - - // If autoscale is not set, we need to send NumWorkers because one of them is required. - // If NumWorkers is not nil, we don't need to set it to ForceSendFields as it will be sent anyway. +// forceNumWorkers ensures NumWorkers is sent when Autoscale is not set, +// because the API requires one of them. +func forceNumWorkers(config *compute.ClusterSpec, fsf *[]string) { if config.Autoscale == nil && config.NumWorkers == 0 { - create.ForceSendFields = append(create.ForceSendFields, "NumWorkers") + *fsf = append(*fsf, "NumWorkers") } - - return create -} - -func makeEditCluster(id string, config *compute.ClusterSpec) compute.EditCluster { - edit := compute.EditCluster{ - ClusterId: id, - ApplyPolicyDefaultValues: config.ApplyPolicyDefaultValues, - Autoscale: config.Autoscale, - AutoterminationMinutes: config.AutoterminationMinutes, - AwsAttributes: config.AwsAttributes, - AzureAttributes: config.AzureAttributes, - ClusterLogConf: config.ClusterLogConf, - ClusterName: config.ClusterName, - CustomTags: config.CustomTags, - DataSecurityMode: config.DataSecurityMode, - DockerImage: config.DockerImage, - DriverInstancePoolId: config.DriverInstancePoolId, - DriverNodeTypeId: config.DriverNodeTypeId, - DriverNodeTypeFlexibility: config.DriverNodeTypeFlexibility, - EnableElasticDisk: config.EnableElasticDisk, - EnableLocalDiskEncryption: config.EnableLocalDiskEncryption, - GcpAttributes: config.GcpAttributes, - InitScripts: config.InitScripts, - InstancePoolId: config.InstancePoolId, - IsSingleNode: config.IsSingleNode, - Kind: config.Kind, - NodeTypeId: config.NodeTypeId, - NumWorkers: config.NumWorkers, - PolicyId: config.PolicyId, - RemoteDiskThroughput: config.RemoteDiskThroughput, - RuntimeEngine: config.RuntimeEngine, - SingleUserName: config.SingleUserName, - SparkConf: config.SparkConf, - SparkEnvVars: config.SparkEnvVars, - SparkVersion: config.SparkVersion, - SshPublicKeys: config.SshPublicKeys, - TotalInitialRemoteDiskSize: config.TotalInitialRemoteDiskSize, - UseMlRuntime: config.UseMlRuntime, - WorkloadType: config.WorkloadType, - WorkerNodeTypeFlexibility: config.WorkerNodeTypeFlexibility, - ForceSendFields: utils.FilterFields[compute.EditCluster](config.ForceSendFields), - } - - // If autoscale is not set, we need to send NumWorkers because one of them is required. - // If NumWorkers is not nil, we don't need to set it to ForceSendFields as it will be sent anyway. - if config.Autoscale == nil && config.NumWorkers == 0 { - edit.ForceSendFields = append(edit.ForceSendFields, "NumWorkers") - } - - return edit } diff --git a/bundle/direct/dresources/external_location.go b/bundle/direct/dresources/external_location.go index ca6821611c..2e00e397fa 100644 --- a/bundle/direct/dresources/external_location.go +++ b/bundle/direct/dresources/external_location.go @@ -4,7 +4,6 @@ import ( "context" "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/libs/utils" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/catalog" ) @@ -21,22 +20,11 @@ func (*ResourceExternalLocation) PrepareState(input *resources.ExternalLocation) return &input.CreateExternalLocation } +// externalLocationRemapCopy maps ExternalLocationInfo (remote GET response) to CreateExternalLocation (local state). +var externalLocationRemapCopy = newCopy[catalog.ExternalLocationInfo, catalog.CreateExternalLocation]() + func (*ResourceExternalLocation) RemapState(info *catalog.ExternalLocationInfo) *catalog.CreateExternalLocation { - return &catalog.CreateExternalLocation{ - Comment: info.Comment, - CredentialName: info.CredentialName, - // Output-only field mirrored into state to avoid churn in remapped config. - EffectiveEnableFileEvents: info.EffectiveEnableFileEvents, - EnableFileEvents: info.EnableFileEvents, - EncryptionDetails: info.EncryptionDetails, - Fallback: info.Fallback, - FileEventQueue: info.FileEventQueue, - Name: info.Name, - ReadOnly: info.ReadOnly, - SkipValidation: false, // This is an input-only parameter, never returned by API - Url: info.Url, - ForceSendFields: utils.FilterFields[catalog.CreateExternalLocation](info.ForceSendFields), - } + return externalLocationRemapCopy.Do(info) } func (r *ResourceExternalLocation) DoRead(ctx context.Context, id string) (*catalog.ExternalLocationInfo, error) { @@ -51,58 +39,27 @@ func (r *ResourceExternalLocation) DoCreate(ctx context.Context, config *catalog return response.Name, response, nil } +// externalLocationUpdateCopy maps CreateExternalLocation (local state) to UpdateExternalLocation (API request). +var externalLocationUpdateCopy = newCopy[catalog.CreateExternalLocation, catalog.UpdateExternalLocation]() + // DoUpdate updates the external location in place and returns remote state. func (r *ResourceExternalLocation) DoUpdate(ctx context.Context, id string, config *catalog.CreateExternalLocation, _ Changes) (*catalog.ExternalLocationInfo, error) { - updateRequest := catalog.UpdateExternalLocation{ - Comment: config.Comment, - CredentialName: config.CredentialName, - // Output-only field; never sent in update payload. - EffectiveEnableFileEvents: false, - EnableFileEvents: config.EnableFileEvents, - EncryptionDetails: config.EncryptionDetails, - Fallback: config.Fallback, - FileEventQueue: config.FileEventQueue, - Force: false, - IsolationMode: "", // Not supported by DABs - Name: id, - NewName: "", // Only set if name actually changes (see DoUpdateWithID) - Owner: "", // Not supported by DABs - ReadOnly: config.ReadOnly, - SkipValidation: config.SkipValidation, - Url: config.Url, - ForceSendFields: utils.FilterFields[catalog.UpdateExternalLocation](config.ForceSendFields, "IsolationMode", "Owner"), - } + updateRequest := externalLocationUpdateCopy.Do(config) + updateRequest.Name = id - return r.client.ExternalLocations.Update(ctx, updateRequest) + return r.client.ExternalLocations.Update(ctx, *updateRequest) } // DoUpdateWithID updates the external location and returns the new ID if the name changes. func (r *ResourceExternalLocation) DoUpdateWithID(ctx context.Context, id string, config *catalog.CreateExternalLocation) (string, *catalog.ExternalLocationInfo, error) { - updateRequest := catalog.UpdateExternalLocation{ - Comment: config.Comment, - CredentialName: config.CredentialName, - // Output-only field; never sent in update payload. - EffectiveEnableFileEvents: false, - EnableFileEvents: config.EnableFileEvents, - EncryptionDetails: config.EncryptionDetails, - Fallback: config.Fallback, - FileEventQueue: config.FileEventQueue, - Force: false, - IsolationMode: "", // Not supported by DABs - Name: id, - NewName: "", // Initialized below if needed - Owner: "", // Not supported by DABs - ReadOnly: config.ReadOnly, - SkipValidation: config.SkipValidation, - Url: config.Url, - ForceSendFields: utils.FilterFields[catalog.UpdateExternalLocation](config.ForceSendFields, "IsolationMode", "Owner"), - } + updateRequest := externalLocationUpdateCopy.Do(config) + updateRequest.Name = id if config.Name != id { updateRequest.NewName = config.Name } - response, err := r.client.ExternalLocations.Update(ctx, updateRequest) + response, err := r.client.ExternalLocations.Update(ctx, *updateRequest) if err != nil { return "", nil, err } diff --git a/bundle/direct/dresources/fieldcopy.go b/bundle/direct/dresources/fieldcopy.go new file mode 100644 index 0000000000..07684845db --- /dev/null +++ b/bundle/direct/dresources/fieldcopy.go @@ -0,0 +1,16 @@ +package dresources + +import "github.com/databricks/cli/libs/structs/fieldcopy" + +type fieldCopyReporter interface { + Report() string +} + +var allFieldCopies []fieldCopyReporter + +func newCopy[Src, Dst any]() *fieldcopy.Copy[Src, Dst] { + c := &fieldcopy.Copy[Src, Dst]{} + c.Init() + allFieldCopies = append(allFieldCopies, c) + return c +} diff --git a/bundle/direct/dresources/fieldcopy_test.go b/bundle/direct/dresources/fieldcopy_test.go new file mode 100644 index 0000000000..41ae0c81b6 --- /dev/null +++ b/bundle/direct/dresources/fieldcopy_test.go @@ -0,0 +1,21 @@ +package dresources + +import ( + "context" + "strings" + "testing" + + "github.com/databricks/cli/libs/testdiff" +) + +func TestFieldCopyReport(t *testing.T) { + ctx := context.Background() + ctx, _ = testdiff.WithReplacementsMap(ctx) + + var buf strings.Builder + for _, c := range allFieldCopies { + buf.WriteString(c.Report()) + } + + testdiff.AssertOutput(t, ctx, buf.String(), "fieldcopy report", "out.fieldcopy.txt") +} diff --git a/bundle/direct/dresources/job.go b/bundle/direct/dresources/job.go index ba3e932b34..f4d98e5664 100644 --- a/bundle/direct/dresources/job.go +++ b/bundle/direct/dresources/job.go @@ -6,7 +6,6 @@ import ( "strconv" "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/libs/utils" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/jobs" @@ -100,12 +99,11 @@ func makeJobRemote(job *jobs.Job) *JobRemote { } } +// jobCreateCopy maps JobSettings (local state) to CreateJob (API request). +var jobCreateCopy = newCopy[jobs.JobSettings, jobs.CreateJob]() + func (r *ResourceJob) DoCreate(ctx context.Context, config *jobs.JobSettings) (string, *JobRemote, error) { - request, err := makeCreateJob(*config) - if err != nil { - return "", nil, err - } - response, err := r.client.Jobs.Create(ctx, request) + response, err := r.client.Jobs.Create(ctx, *jobCreateCopy.Do(config)) if err != nil { return "", nil, err } @@ -113,11 +111,14 @@ func (r *ResourceJob) DoCreate(ctx context.Context, config *jobs.JobSettings) (s } func (r *ResourceJob) DoUpdate(ctx context.Context, id string, config *jobs.JobSettings, _ Changes) (*JobRemote, error) { - request, err := makeResetJob(*config, id) + idInt, err := parseJobID(id) if err != nil { return nil, err } - return nil, r.client.Jobs.Reset(ctx, request) + return nil, r.client.Jobs.Reset(ctx, jobs.ResetJob{ + JobId: idInt, + NewSettings: *config, + }) } func (r *ResourceJob) DoDelete(ctx context.Context, id string) error { @@ -128,54 +129,6 @@ func (r *ResourceJob) DoDelete(ctx context.Context, id string) error { return r.client.Jobs.DeleteByJobId(ctx, idInt) } -func makeCreateJob(config jobs.JobSettings) (jobs.CreateJob, error) { - // Note, exhaustruct linter validates that all off CreateJob fields are initialized. - // We don't have linter that validates that all of config fields are used. - result := jobs.CreateJob{ - AccessControlList: nil, // Not supported by DABs - BudgetPolicyId: config.BudgetPolicyId, - Continuous: config.Continuous, - Deployment: config.Deployment, - Description: config.Description, - EditMode: config.EditMode, - EmailNotifications: config.EmailNotifications, - Environments: config.Environments, - Format: config.Format, - GitSource: config.GitSource, - Health: config.Health, - JobClusters: config.JobClusters, - MaxConcurrentRuns: config.MaxConcurrentRuns, - Name: config.Name, - NotificationSettings: config.NotificationSettings, - Parameters: config.Parameters, - PerformanceTarget: config.PerformanceTarget, - Queue: config.Queue, - RunAs: config.RunAs, - Schedule: config.Schedule, - Tags: config.Tags, - Tasks: config.Tasks, - TimeoutSeconds: config.TimeoutSeconds, - Trigger: config.Trigger, - UsagePolicyId: config.UsagePolicyId, - WebhookNotifications: config.WebhookNotifications, - ForceSendFields: utils.FilterFields[jobs.CreateJob](config.ForceSendFields, "AccessControlList"), - } - - return result, nil -} - -func makeResetJob(config jobs.JobSettings, id string) (jobs.ResetJob, error) { - idInt, err := parseJobID(id) - if err != nil { - return jobs.ResetJob{}, err - } - result := jobs.ResetJob{ - JobId: idInt, - NewSettings: config, - } - return result, err -} - func parseJobID(id string) (int64, error) { result, err := strconv.ParseInt(id, 10, 64) if err != nil { diff --git a/bundle/direct/dresources/model_serving_endpoint.go b/bundle/direct/dresources/model_serving_endpoint.go index 8a8aa126b2..cc657e392e 100644 --- a/bundle/direct/dresources/model_serving_endpoint.go +++ b/bundle/direct/dresources/model_serving_endpoint.go @@ -7,7 +7,6 @@ import ( "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/libs/structs/structpath" - "github.com/databricks/cli/libs/utils" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/serving" ) @@ -34,42 +33,24 @@ func (*ResourceModelServingEndpoint) PrepareState(input *resources.ModelServingE return &input.CreateServingEndpoint } +// autoCaptureConfigCopy maps AutoCaptureConfigOutput to AutoCaptureConfigInput. +var autoCaptureConfigCopy = newCopy[serving.AutoCaptureConfigOutput, serving.AutoCaptureConfigInput]() + func autoCaptureConfigOutputToInput(output *serving.AutoCaptureConfigOutput) *serving.AutoCaptureConfigInput { if output == nil { return nil } - return &serving.AutoCaptureConfigInput{ - CatalogName: output.CatalogName, - SchemaName: output.SchemaName, - TableNamePrefix: output.TableNamePrefix, - Enabled: output.Enabled, - ForceSendFields: utils.FilterFields[serving.AutoCaptureConfigInput](output.ForceSendFields), - } + return autoCaptureConfigCopy.Do(output) } +// servedEntityCopy maps ServedEntityOutput to ServedEntityInput. +var servedEntityCopy = newCopy[serving.ServedEntityOutput, serving.ServedEntityInput]() + func servedEntitiesOutputToInput(output []serving.ServedEntityOutput) []serving.ServedEntityInput { entities := make([]serving.ServedEntityInput, len(output)) for i, entity := range output { - entities[i] = serving.ServedEntityInput{ - EntityName: entity.EntityName, - EntityVersion: entity.EntityVersion, - EnvironmentVars: entity.EnvironmentVars, - ExternalModel: entity.ExternalModel, - InstanceProfileArn: entity.InstanceProfileArn, - MaxProvisionedConcurrency: entity.MaxProvisionedConcurrency, - MaxProvisionedThroughput: entity.MaxProvisionedThroughput, - MinProvisionedConcurrency: entity.MinProvisionedConcurrency, - MinProvisionedThroughput: entity.MinProvisionedThroughput, - Name: entity.Name, - ProvisionedModelUnits: entity.ProvisionedModelUnits, - ScaleToZeroEnabled: entity.ScaleToZeroEnabled, - WorkloadSize: entity.WorkloadSize, - WorkloadType: entity.WorkloadType, - BurstScalingEnabled: entity.BurstScalingEnabled, - ForceSendFields: utils.FilterFields[serving.ServedEntityInput](entity.ForceSendFields), - } + entities[i] = *servedEntityCopy.Do(&entity) } - return entities } @@ -86,25 +67,14 @@ func configOutputToInput(output *serving.EndpointCoreConfigOutput) *serving.Endp } } +// servingRemapCopy maps ServingEndpointDetailed (remote GET response) to CreateServingEndpoint (local state). +var servingRemapCopy = newCopy[serving.ServingEndpointDetailed, serving.CreateServingEndpoint]() + func (*ResourceModelServingEndpoint) RemapState(state *RefreshOutput) *serving.CreateServingEndpoint { details := state.EndpointDetails - // Map the remote state (ServingEndpointDetailed) to the local state (CreateServingEndpoint) - // for proper comparison during diff calculation - return &serving.CreateServingEndpoint{ - AiGateway: details.AiGateway, - BudgetPolicyId: details.BudgetPolicyId, - Config: configOutputToInput(details.Config), - Description: details.Description, - EmailNotifications: details.EmailNotifications, - Name: details.Name, - RouteOptimized: details.RouteOptimized, - Tags: details.Tags, - ForceSendFields: utils.FilterFields[serving.CreateServingEndpoint](details.ForceSendFields), - - // Rate limits are a deprecated field that are not returned by the API on GET calls. Thus we map them to nil. - // TODO(shreyas): Add a warning when users try setting top level rate limits. - RateLimits: nil, - } + result := servingRemapCopy.Do(details) + result.Config = configOutputToInput(details.Config) + return result } type RefreshOutput struct { diff --git a/bundle/direct/dresources/out.fieldcopy.txt b/bundle/direct/dresources/out.fieldcopy.txt new file mode 100644 index 0000000000..69663784c6 --- /dev/null +++ b/bundle/direct/dresources/out.fieldcopy.txt @@ -0,0 +1,162 @@ +catalog.CatalogInfo → catalog.CreateCatalog + src not copied: + - BrowseOnly + - CatalogType + - CreatedAt + - CreatedBy + - EffectivePredictiveOptimizationFlag + - EnablePredictiveOptimization + - FullName + - IsolationMode + - MetastoreId + - Owner + - ProvisioningInfo + - SecurableType + - StorageLocation + - UpdatedAt + - UpdatedBy +catalog.CreateCatalog → catalog.UpdateCatalog + src not copied: + - ConnectionName + - ProviderName + - ShareName + - StorageRoot + dst not set: + - EnablePredictiveOptimization + - IsolationMode + - NewName + - Owner +compute.ClusterDetails → compute.ClusterSpec + src not copied: + - ClusterCores + - ClusterId + - ClusterLogStatus + - ClusterMemoryMb + - ClusterSource + - CreatorUserName + - DefaultTags + - Driver + - Executors + - JdbcPort + - LastRestartedTime + - LastStateLossTime + - SparkContextId + - Spec + - StartTime + - State + - StateMessage + - TerminatedTime + - TerminationReason + dst not set: + - ApplyPolicyDefaultValues +compute.ClusterSpec → compute.CreateCluster + dst not set: + - CloneFrom +compute.ClusterSpec → compute.EditCluster + dst not set: + - ClusterId +catalog.ExternalLocationInfo → catalog.CreateExternalLocation + src not copied: + - BrowseOnly + - CreatedAt + - CreatedBy + - CredentialId + - IsolationMode + - MetastoreId + - Owner + - UpdatedAt + - UpdatedBy + dst not set: + - SkipValidation +catalog.CreateExternalLocation → catalog.UpdateExternalLocation + dst not set: + - Force + - IsolationMode + - NewName + - Owner +jobs.JobSettings → jobs.CreateJob + dst not set: + - AccessControlList +serving.AutoCaptureConfigOutput → serving.AutoCaptureConfigInput + src not copied: + - State +serving.ServedEntityOutput → serving.ServedEntityInput + src not copied: + - CreationTimestamp + - Creator + - FoundationModel + - State +serving.ServingEndpointDetailed → serving.CreateServingEndpoint + src not copied: + - Config + - CreationTimestamp + - Creator + - DataPlaneInfo + - EndpointUrl + - Id + - LastUpdatedTimestamp + - PendingConfig + - PermissionLevel + - State + - Task + dst not set: + - Config + - RateLimits +pipelines.PipelineSpec → pipelines.CreatePipeline + dst not set: + - AllowDuplicateNames + - DryRun + - RunAs +pipelines.GetPipelineResponse → dresources.PipelineRemote + src not copied: + - Name + - RunAs + - Spec + - ForceSendFields + dst not set: + - CreatePipeline +pipelines.CreatePipeline → pipelines.EditPipeline + src not copied: + - DryRun + dst not set: + - ExpectedLastModified + - PipelineId +catalog.MonitorInfo → catalog.CreateMonitor + src not copied: + - DashboardId + - DriftMetricsTableName + - MonitorVersion + - ProfileMetricsTableName + - Status + dst not set: + - SkipBuiltinDashboard + - WarehouseId +catalog.CreateMonitor → catalog.UpdateMonitor + src not copied: + - AssetsDir + - SkipBuiltinDashboard + - WarehouseId + dst not set: + - DashboardId +catalog.RegisteredModelInfo → catalog.CreateRegisteredModelRequest: all fields matched +catalog.CreateRegisteredModelRequest → catalog.UpdateRegisteredModelRequest + dst not set: + - NewName +sql.GetWarehouseResponse → sql.CreateWarehouseRequest + src not copied: + - Health + - Id + - JdbcUrl + - NumActiveSessions + - NumClusters + - OdbcParams + - State + - WarehouseType + dst not set: + - WarehouseType +sql.CreateWarehouseRequest → sql.EditWarehouseRequest + src not copied: + - WarehouseType + dst not set: + - Id + - WarehouseType diff --git a/bundle/direct/dresources/pipeline.go b/bundle/direct/dresources/pipeline.go index 30d2ab5313..d8e8c6c879 100644 --- a/bundle/direct/dresources/pipeline.go +++ b/bundle/direct/dresources/pipeline.go @@ -4,7 +4,6 @@ import ( "context" "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/libs/utils" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/pipelines" @@ -65,61 +64,19 @@ func (r *ResourcePipeline) DoRead(ctx context.Context, id string) (*PipelineRemo return makePipelineRemote(resp), nil } +// pipelineSpecCopy maps PipelineSpec (from GET response) to CreatePipeline (local state). +var pipelineSpecCopy = newCopy[pipelines.PipelineSpec, pipelines.CreatePipeline]() + +// pipelineRemoteCopy maps GetPipelineResponse to PipelineRemote extra fields. +var pipelineRemoteCopy = newCopy[pipelines.GetPipelineResponse, PipelineRemote]() + func makePipelineRemote(p *pipelines.GetPipelineResponse) *PipelineRemote { - var createPipeline pipelines.CreatePipeline + remote := pipelineRemoteCopy.Do(p) if p.Spec != nil { - spec := p.Spec - createPipeline = pipelines.CreatePipeline{ - // Note: AllowDuplicateNames and DryRun are not in PipelineSpec, - // they are request-only fields, so they stay at their zero values. - AllowDuplicateNames: false, - BudgetPolicyId: spec.BudgetPolicyId, - Catalog: spec.Catalog, - Channel: spec.Channel, - Clusters: spec.Clusters, - Configuration: spec.Configuration, - Continuous: spec.Continuous, - Deployment: spec.Deployment, - Development: spec.Development, - DryRun: false, - Edition: spec.Edition, - Environment: spec.Environment, - EventLog: spec.EventLog, - Filters: spec.Filters, - GatewayDefinition: spec.GatewayDefinition, - Id: spec.Id, - IngestionDefinition: spec.IngestionDefinition, - Libraries: spec.Libraries, - Name: spec.Name, - Notifications: spec.Notifications, - Photon: spec.Photon, - RestartWindow: spec.RestartWindow, - RootPath: spec.RootPath, - RunAs: p.RunAs, - Schema: spec.Schema, - Serverless: spec.Serverless, - Storage: spec.Storage, - Tags: spec.Tags, - Target: spec.Target, - Trigger: spec.Trigger, - UsagePolicyId: spec.UsagePolicyId, - ForceSendFields: utils.FilterFields[pipelines.CreatePipeline](spec.ForceSendFields, "AllowDuplicateNames", "DryRun", "RunAs"), - } - } - return &PipelineRemote{ - CreatePipeline: createPipeline, - Cause: p.Cause, - ClusterId: p.ClusterId, - CreatorUserName: p.CreatorUserName, - EffectiveBudgetPolicyId: p.EffectiveBudgetPolicyId, - EffectivePublishingMode: p.EffectivePublishingMode, - Health: p.Health, - LastModified: p.LastModified, - LatestUpdates: p.LatestUpdates, - PipelineId: p.PipelineId, - RunAsUserName: p.RunAsUserName, - State: p.State, + remote.CreatePipeline = *pipelineSpecCopy.Do(p.Spec) + remote.CreatePipeline.RunAs = p.RunAs } + return remote } func (r *ResourcePipeline) DoCreate(ctx context.Context, config *pipelines.CreatePipeline) (string, *PipelineRemote, error) { @@ -130,44 +87,13 @@ func (r *ResourcePipeline) DoCreate(ctx context.Context, config *pipelines.Creat return response.PipelineId, nil, nil } -func (r *ResourcePipeline) DoUpdate(ctx context.Context, id string, config *pipelines.CreatePipeline, _ Changes) (*PipelineRemote, error) { - request := pipelines.EditPipeline{ - AllowDuplicateNames: config.AllowDuplicateNames, - BudgetPolicyId: config.BudgetPolicyId, - Catalog: config.Catalog, - Channel: config.Channel, - Clusters: config.Clusters, - Configuration: config.Configuration, - Continuous: config.Continuous, - Deployment: config.Deployment, - Development: config.Development, - Edition: config.Edition, - Environment: config.Environment, - EventLog: config.EventLog, - ExpectedLastModified: 0, - Filters: config.Filters, - GatewayDefinition: config.GatewayDefinition, - Id: config.Id, - IngestionDefinition: config.IngestionDefinition, - Libraries: config.Libraries, - Name: config.Name, - Notifications: config.Notifications, - Photon: config.Photon, - RestartWindow: config.RestartWindow, - RootPath: config.RootPath, - RunAs: config.RunAs, - Schema: config.Schema, - Serverless: config.Serverless, - Storage: config.Storage, - Tags: config.Tags, - Target: config.Target, - Trigger: config.Trigger, - UsagePolicyId: config.UsagePolicyId, - PipelineId: id, - ForceSendFields: utils.FilterFields[pipelines.EditPipeline](config.ForceSendFields), - } +// pipelineEditCopy maps CreatePipeline (local state) to EditPipeline (API request). +var pipelineEditCopy = newCopy[pipelines.CreatePipeline, pipelines.EditPipeline]() - return nil, r.client.Pipelines.Update(ctx, request) +func (r *ResourcePipeline) DoUpdate(ctx context.Context, id string, config *pipelines.CreatePipeline, _ Changes) (*PipelineRemote, error) { + request := pipelineEditCopy.Do(config) + request.PipelineId = id + return nil, r.client.Pipelines.Update(ctx, *request) } func (r *ResourcePipeline) DoDelete(ctx context.Context, id string) error { diff --git a/bundle/direct/dresources/quality_monitor.go b/bundle/direct/dresources/quality_monitor.go index dcc48fb95d..4de56ec50d 100644 --- a/bundle/direct/dresources/quality_monitor.go +++ b/bundle/direct/dresources/quality_monitor.go @@ -4,7 +4,6 @@ import ( "context" "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/libs/utils" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/catalog" @@ -41,27 +40,13 @@ func (*ResourceQualityMonitor) PrepareState(input *resources.QualityMonitor) *Qu } } +// qualityMonitorRemapCopy maps MonitorInfo (remote GET response) to CreateMonitor (local state). +var qualityMonitorRemapCopy = newCopy[catalog.MonitorInfo, catalog.CreateMonitor]() + func (*ResourceQualityMonitor) RemapState(info *catalog.MonitorInfo) *QualityMonitorState { return &QualityMonitorState{ - CreateMonitor: catalog.CreateMonitor{ - AssetsDir: info.AssetsDir, - BaselineTableName: info.BaselineTableName, - CustomMetrics: info.CustomMetrics, - DataClassificationConfig: info.DataClassificationConfig, - InferenceLog: info.InferenceLog, - LatestMonitorFailureMsg: info.LatestMonitorFailureMsg, - Notifications: info.Notifications, - OutputSchemaName: info.OutputSchemaName, - Schedule: info.Schedule, - SkipBuiltinDashboard: false, - SlicingExprs: info.SlicingExprs, - Snapshot: info.Snapshot, - TableName: info.TableName, - TimeSeries: info.TimeSeries, - WarehouseId: "", - ForceSendFields: utils.FilterFields[catalog.CreateMonitor](info.ForceSendFields), - }, - TableName: info.TableName, + CreateMonitor: *qualityMonitorRemapCopy.Do(info), + TableName: info.TableName, } } @@ -83,26 +68,15 @@ func (r *ResourceQualityMonitor) DoCreate(ctx context.Context, config *QualityMo return response.TableName, response, nil } +// qualityMonitorUpdateCopy maps CreateMonitor (local state) to UpdateMonitor (API request). +var qualityMonitorUpdateCopy = newCopy[catalog.CreateMonitor, catalog.UpdateMonitor]() + func (r *ResourceQualityMonitor) DoUpdate(ctx context.Context, id string, config *QualityMonitorState, _ Changes) (*catalog.MonitorInfo, error) { - updateRequest := catalog.UpdateMonitor{ - TableName: id, - BaselineTableName: config.BaselineTableName, - CustomMetrics: config.CustomMetrics, - DashboardId: "", - DataClassificationConfig: config.DataClassificationConfig, - InferenceLog: config.InferenceLog, - LatestMonitorFailureMsg: "", - Notifications: config.Notifications, - OutputSchemaName: config.OutputSchemaName, - Schedule: config.Schedule, - SlicingExprs: config.SlicingExprs, - Snapshot: config.Snapshot, - TimeSeries: config.TimeSeries, - ForceSendFields: utils.FilterFields[catalog.UpdateMonitor](config.ForceSendFields), - } + updateRequest := qualityMonitorUpdateCopy.Do(&config.CreateMonitor) + updateRequest.TableName = id //nolint:staticcheck // Direct quality_monitor resource still uses legacy monitor endpoints; v1 data-quality migration is separate work. - response, err := r.client.QualityMonitors.Update(ctx, updateRequest) + response, err := r.client.QualityMonitors.Update(ctx, *updateRequest) if err != nil { return nil, err } diff --git a/bundle/direct/dresources/registered_model.go b/bundle/direct/dresources/registered_model.go index 666716b48b..2a9ef85e7e 100644 --- a/bundle/direct/dresources/registered_model.go +++ b/bundle/direct/dresources/registered_model.go @@ -4,7 +4,6 @@ import ( "context" "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/libs/utils" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/catalog" ) @@ -23,27 +22,11 @@ func (*ResourceRegisteredModel) PrepareState(input *resources.RegisteredModel) * return &input.CreateRegisteredModelRequest } -func (*ResourceRegisteredModel) RemapState(model *catalog.RegisteredModelInfo) *catalog.CreateRegisteredModelRequest { - return &catalog.CreateRegisteredModelRequest{ - CatalogName: model.CatalogName, - Comment: model.Comment, - Name: model.Name, - SchemaName: model.SchemaName, - StorageLocation: model.StorageLocation, - ForceSendFields: utils.FilterFields[catalog.CreateRegisteredModelRequest](model.ForceSendFields), - - Aliases: model.Aliases, - BrowseOnly: model.BrowseOnly, - FullName: model.FullName, - MetastoreId: model.MetastoreId, - Owner: model.Owner, +// registeredModelRemapCopy maps RegisteredModelInfo (remote GET response) to CreateRegisteredModelRequest (local state). +var registeredModelRemapCopy = newCopy[catalog.RegisteredModelInfo, catalog.CreateRegisteredModelRequest]() - // Clear output only fields. They should not show up on remote diff computation. - CreatedAt: 0, - CreatedBy: "", - UpdatedAt: 0, - UpdatedBy: "", - } +func (*ResourceRegisteredModel) RemapState(model *catalog.RegisteredModelInfo) *catalog.CreateRegisteredModelRequest { + return registeredModelRemapCopy.Do(model) } func (r *ResourceRegisteredModel) DoRead(ctx context.Context, id string) (*catalog.RegisteredModelInfo, error) { @@ -64,33 +47,14 @@ func (r *ResourceRegisteredModel) DoCreate(ctx context.Context, config *catalog. return response.FullName, response, nil } -func (r *ResourceRegisteredModel) DoUpdate(ctx context.Context, id string, config *catalog.CreateRegisteredModelRequest, _ Changes) (*catalog.RegisteredModelInfo, error) { - updateRequest := catalog.UpdateRegisteredModelRequest{ - FullName: id, - Comment: config.Comment, - ForceSendFields: utils.FilterFields[catalog.UpdateRegisteredModelRequest](config.ForceSendFields, "Owner", "NewName"), - - // Owner is not part of the configuration tree - 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: "", +// registeredModelUpdateCopy maps CreateRegisteredModelRequest (local state) to UpdateRegisteredModelRequest (API request). +var registeredModelUpdateCopy = newCopy[catalog.CreateRegisteredModelRequest, catalog.UpdateRegisteredModelRequest]() - Aliases: config.Aliases, - BrowseOnly: config.BrowseOnly, - CreatedAt: config.CreatedAt, - CreatedBy: config.CreatedBy, - MetastoreId: config.MetastoreId, - UpdatedAt: config.UpdatedAt, - UpdatedBy: config.UpdatedBy, - SchemaName: config.SchemaName, - StorageLocation: config.StorageLocation, - Name: config.Name, - CatalogName: config.CatalogName, - } +func (r *ResourceRegisteredModel) DoUpdate(ctx context.Context, id string, config *catalog.CreateRegisteredModelRequest, _ Changes) (*catalog.RegisteredModelInfo, error) { + updateRequest := registeredModelUpdateCopy.Do(config) + updateRequest.FullName = id - response, err := r.client.RegisteredModels.Update(ctx, updateRequest) + response, err := r.client.RegisteredModels.Update(ctx, *updateRequest) if err != nil { return nil, err } diff --git a/bundle/direct/dresources/sql_warehouse.go b/bundle/direct/dresources/sql_warehouse.go index 5d9d7793b7..8929164274 100644 --- a/bundle/direct/dresources/sql_warehouse.go +++ b/bundle/direct/dresources/sql_warehouse.go @@ -5,7 +5,6 @@ import ( "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/libs/log" - "github.com/databricks/cli/libs/utils" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/sql" ) @@ -24,23 +23,14 @@ func (*ResourceSqlWarehouse) PrepareState(input *resources.SqlWarehouse) *sql.Cr return &input.CreateWarehouseRequest } +// sqlWarehouseRemapCopy maps GetWarehouseResponse (remote GET response) to CreateWarehouseRequest (local state). +var sqlWarehouseRemapCopy = newCopy[sql.GetWarehouseResponse, sql.CreateWarehouseRequest]() + func (*ResourceSqlWarehouse) RemapState(warehouse *sql.GetWarehouseResponse) *sql.CreateWarehouseRequest { - return &sql.CreateWarehouseRequest{ - AutoStopMins: warehouse.AutoStopMins, - Channel: warehouse.Channel, - ClusterSize: warehouse.ClusterSize, - CreatorName: warehouse.CreatorName, - EnablePhoton: warehouse.EnablePhoton, - EnableServerlessCompute: warehouse.EnableServerlessCompute, - InstanceProfileArn: warehouse.InstanceProfileArn, - MaxNumClusters: warehouse.MaxNumClusters, - MinNumClusters: warehouse.MinNumClusters, - Name: warehouse.Name, - SpotInstancePolicy: warehouse.SpotInstancePolicy, - Tags: warehouse.Tags, - WarehouseType: sql.CreateWarehouseRequestWarehouseType(warehouse.WarehouseType), - ForceSendFields: utils.FilterFields[sql.CreateWarehouseRequest](warehouse.ForceSendFields), - } + result := sqlWarehouseRemapCopy.Do(warehouse) + // WarehouseType requires explicit conversion between different named string types. + result.WarehouseType = sql.CreateWarehouseRequestWarehouseType(warehouse.WarehouseType) + return result } // DoRead reads the warehouse by id. @@ -57,27 +47,17 @@ func (r *ResourceSqlWarehouse) DoCreate(ctx context.Context, config *sql.CreateW return waiter.Id, nil, nil } +// sqlWarehouseEditCopy maps CreateWarehouseRequest (local state) to EditWarehouseRequest (API request). +var sqlWarehouseEditCopy = newCopy[sql.CreateWarehouseRequest, sql.EditWarehouseRequest]() + // DoUpdate updates the warehouse in place. func (r *ResourceSqlWarehouse) DoUpdate(ctx context.Context, id string, config *sql.CreateWarehouseRequest, _ Changes) (*sql.GetWarehouseResponse, error) { - request := sql.EditWarehouseRequest{ - AutoStopMins: config.AutoStopMins, - Channel: config.Channel, - ClusterSize: config.ClusterSize, - CreatorName: config.CreatorName, - EnablePhoton: config.EnablePhoton, - EnableServerlessCompute: config.EnableServerlessCompute, - Id: id, - InstanceProfileArn: config.InstanceProfileArn, - MaxNumClusters: config.MaxNumClusters, - MinNumClusters: config.MinNumClusters, - Name: config.Name, - SpotInstancePolicy: config.SpotInstancePolicy, - Tags: config.Tags, - WarehouseType: sql.EditWarehouseRequestWarehouseType(config.WarehouseType), - ForceSendFields: utils.FilterFields[sql.EditWarehouseRequest](config.ForceSendFields), - } + request := sqlWarehouseEditCopy.Do(config) + request.Id = id + // WarehouseType requires explicit conversion between different named string types. + request.WarehouseType = sql.EditWarehouseRequestWarehouseType(config.WarehouseType) - waiter, err := r.client.Warehouses.Edit(ctx, request) + waiter, err := r.client.Warehouses.Edit(ctx, *request) if err != nil { return nil, err } diff --git a/libs/structs/fieldcopy/fieldcopy.go b/libs/structs/fieldcopy/fieldcopy.go new file mode 100644 index 0000000000..fa02d59221 --- /dev/null +++ b/libs/structs/fieldcopy/fieldcopy.go @@ -0,0 +1,261 @@ +// Package fieldcopy provides reflection-based struct-to-struct field copying +// with test-time completeness validation via golden files. +// +// Field mappings are computed once on first use and cached for subsequent calls. +// If both Src and Dst have a ForceSendFields []string field, it is handled +// automatically: source values are filtered to only include names of exported +// Dst fields that are being copied. +package fieldcopy + +import ( + "fmt" + "reflect" + "sort" + "strings" +) + +const forceSendFieldsName = "ForceSendFields" + +// Copy describes a field-by-field mapping from Src to Dst struct type. +// Fields with matching names and assignable types are copied automatically. +// Unmatched fields are left at zero and reported via [Copy.Report] for +// golden file testing. +// +// Call [Copy.Init] before using [Copy.Do]; Do panics if Init was not called. +type Copy[Src, Dst any] struct { + // Rename maps destination field name to source field name for fields + // with different names in source and destination. + Rename map[string]string + + // ops is the list of field copy operations (src index → dst index). + ops []fieldOp + + // autoFSF is true when ForceSendFields is auto-handled. + autoFSF bool + + // fsfSrcIdx is the reflect field index of ForceSendFields on Src. + fsfSrcIdx []int + + // fsfDstIdx is the reflect field index of ForceSendFields on Dst. + fsfDstIdx []int + + // validFSFNames is the set of dst field names eligible for ForceSendFields. + validFSFNames map[string]bool + + // ready is set to true after Init completes. + ready bool +} + +// Init eagerly computes field mappings via reflection. Must be called before Do. +func (c *Copy[Src, Dst]) Init() { + c.build() +} + +// Do copies fields from src to a new Dst value using precomputed field mappings. +// Panics if [Copy.Init] was not called. +func (c *Copy[Src, Dst]) Do(src *Src) *Dst { + if !c.ready { + panic(fmt.Sprintf("fieldcopy: Do called on uninitialized Copy[%v, %v]; call Init first", reflect.TypeFor[Src](), reflect.TypeFor[Dst]())) + } + var dst Dst + sv := reflect.ValueOf(src).Elem() + dv := reflect.ValueOf(&dst).Elem() + for _, op := range c.ops { + dv.FieldByIndex(op.dstIndex).Set(sv.FieldByIndex(op.srcIndex)) + } + if c.autoFSF { + srcFSF := sv.FieldByIndex(c.fsfSrcIdx) + if !srcFSF.IsNil() { + srcFields := srcFSF.Interface().([]string) + var filtered []string + for _, name := range srcFields { + if c.validFSFNames[name] { + filtered = append(filtered, name) + } + } + if filtered != nil { + dv.FieldByIndex(c.fsfDstIdx).Set(reflect.ValueOf(filtered)) + } + } + } + return &dst +} + +type fieldOp struct { + dstIndex []int + srcIndex []int +} + +func (c *Copy[Src, Dst]) build() { + srcType := reflect.TypeFor[Src]() + dstType := reflect.TypeFor[Dst]() + + // Detect auto-handling of ForceSendFields: both types must have it as []string. + stringSliceType := reflect.TypeFor[[]string]() + dstFSF, dstOK := dstType.FieldByName(forceSendFieldsName) + srcFSF, srcOK := srcType.FieldByName(forceSendFieldsName) + // Only auto-handle when ForceSendFields is a direct (non-promoted) field on both types. + // Promoted fields from embedded structs have len(Index) > 1. + if dstOK && srcOK && len(dstFSF.Index) == 1 && len(srcFSF.Index) == 1 && dstFSF.Type == stringSliceType && srcFSF.Type == stringSliceType { + c.autoFSF = true + c.fsfSrcIdx = srcFSF.Index + c.fsfDstIdx = dstFSF.Index + c.validFSFNames = make(map[string]bool) + } + + for i := range dstType.NumField() { + df := dstType.Field(i) + if !df.IsExported() { + continue + } + if c.autoFSF && df.Name == forceSendFieldsName { + continue // handled separately + } + + srcName := df.Name + if c.Rename != nil { + if renamed, ok := c.Rename[df.Name]; ok { + srcName = renamed + } + } + + sf, ok := srcType.FieldByName(srcName) + if !ok || !sf.Type.AssignableTo(df.Type) { + continue + } + + c.ops = append(c.ops, fieldOp{dstIndex: df.Index, srcIndex: sf.Index}) + if c.autoFSF { + c.validFSFNames[df.Name] = true + } + } + + c.ready = true +} + +// Report returns a human-readable summary of unmatched fields for golden file testing. +// Fields that exist on Src but have no match on Dst are listed as "src not copied". +// Fields that exist on Dst but have no match on Src are listed as "dst not set". +func (c *Copy[Src, Dst]) Report() string { + srcType := reflect.TypeFor[Src]() + dstType := reflect.TypeFor[Dst]() + return c.report(srcType, dstType) +} + +func (c *Copy[Src, Dst]) report(srcType, dstType reflect.Type) string { + // Build rename lookups. + renameTargets := make(map[string]bool) // src names used by Rename + if c.Rename != nil { + for _, srcName := range c.Rename { + renameTargets[srcName] = true + } + } + + // Detect auto-handled ForceSendFields (direct fields only, not promoted from embedded structs). + autoFSF := false + stringSliceType := reflect.TypeFor[[]string]() + if dstFSF, ok := dstType.FieldByName(forceSendFieldsName); ok { + if srcFSF, ok := srcType.FieldByName(forceSendFieldsName); ok { + if len(dstFSF.Index) == 1 && len(srcFSF.Index) == 1 && dstFSF.Type == stringSliceType && srcFSF.Type == stringSliceType { + autoFSF = true + } + } + } + + // Collect matched dst field names (including renames). + matchedDst := make(map[string]bool) + matchedSrc := make(map[string]bool) + for i := range dstType.NumField() { + df := dstType.Field(i) + if !df.IsExported() { + continue + } + if autoFSF && df.Name == forceSendFieldsName { + matchedDst[df.Name] = true + matchedSrc[df.Name] = true + continue + } + + srcName := df.Name + if c.Rename != nil { + if renamed, ok := c.Rename[df.Name]; ok { + srcName = renamed + } + } + + sf, ok := srcType.FieldByName(srcName) + if ok && sf.Type.AssignableTo(df.Type) { + matchedDst[df.Name] = true + matchedSrc[srcName] = true + } + } + + // Find unmatched src fields. + var unmatchedSrc []string + for i := range srcType.NumField() { + sf := srcType.Field(i) + if !sf.IsExported() || matchedSrc[sf.Name] || renameTargets[sf.Name] { + continue + } + unmatchedSrc = append(unmatchedSrc, sf.Name) + } + + // Find unmatched dst fields. + var unmatchedDst []string + for i := range dstType.NumField() { + df := dstType.Field(i) + if !df.IsExported() || matchedDst[df.Name] { + continue + } + unmatchedDst = append(unmatchedDst, df.Name) + } + + // Check for stale Rename entries. + var staleRenames []string + if c.Rename != nil { + for dstName, srcName := range c.Rename { + var issues []string + if _, ok := dstType.FieldByName(dstName); !ok { + issues = append(issues, fmt.Sprintf("dst %q not found", dstName)) + } + if _, ok := srcType.FieldByName(srcName); !ok { + issues = append(issues, fmt.Sprintf("src %q not found", srcName)) + } + if len(issues) > 0 { + staleRenames = append(staleRenames, fmt.Sprintf("%s→%s (%s)", dstName, srcName, strings.Join(issues, ", "))) + } + } + sort.Strings(staleRenames) + } + + var buf strings.Builder + fmt.Fprintf(&buf, "%v → %v", srcType, dstType) + + if len(unmatchedSrc) == 0 && len(unmatchedDst) == 0 && len(staleRenames) == 0 { + buf.WriteString(": all fields matched\n") + return buf.String() + } + + buf.WriteString("\n") + + if len(unmatchedSrc) > 0 { + buf.WriteString(" src not copied:\n") + for _, name := range unmatchedSrc { + fmt.Fprintf(&buf, " - %s\n", name) + } + } + if len(unmatchedDst) > 0 { + buf.WriteString(" dst not set:\n") + for _, name := range unmatchedDst { + fmt.Fprintf(&buf, " - %s\n", name) + } + } + if len(staleRenames) > 0 { + buf.WriteString(" stale renames:\n") + for _, s := range staleRenames { + fmt.Fprintf(&buf, " - %s\n", s) + } + } + + return buf.String() +} diff --git a/libs/structs/fieldcopy/fieldcopy_test.go b/libs/structs/fieldcopy/fieldcopy_test.go new file mode 100644 index 0000000000..ef0246cbc9 --- /dev/null +++ b/libs/structs/fieldcopy/fieldcopy_test.go @@ -0,0 +1,408 @@ +package fieldcopy_test + +import ( + "testing" + + "github.com/databricks/cli/libs/structs/fieldcopy" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type srcBasic struct { + Name string + Age int + Email string + private string //nolint:unused +} + +type dstBasic struct { + Name string + Age int + Email string +} + +func TestDoBasicCopy(t *testing.T) { + c := fieldcopy.Copy[srcBasic, dstBasic]{} + c.Init() + src := srcBasic{Name: "alice", Age: 30, Email: "a@b.c"} + dst := c.Do(&src) + assert.Equal(t, "alice", dst.Name) + assert.Equal(t, 30, dst.Age) + assert.Equal(t, "a@b.c", dst.Email) +} + +func TestDoCachedSecondCall(t *testing.T) { + c := fieldcopy.Copy[srcBasic, dstBasic]{} + c.Init() + src1 := srcBasic{Name: "alice", Age: 30, Email: "a@b.c"} + dst1 := c.Do(&src1) + assert.Equal(t, "alice", dst1.Name) + + src2 := srcBasic{Name: "bob", Age: 25, Email: "b@c.d"} + dst2 := c.Do(&src2) + assert.Equal(t, "bob", dst2.Name) + assert.Equal(t, 25, dst2.Age) +} + +type srcWithExtra struct { + Name string + Age int + Extra string +} + +type dstSmall struct { + Name string + Age int +} + +func TestDoUnmatchedFieldsIgnored(t *testing.T) { + c := fieldcopy.Copy[srcWithExtra, dstSmall]{} + c.Init() + src := srcWithExtra{Name: "alice", Age: 30, Extra: "ignored"} + dst := c.Do(&src) + assert.Equal(t, "alice", dst.Name) + assert.Equal(t, 30, dst.Age) +} + +type dstWithDefault struct { + Name string + Age int + Default string +} + +func TestDoUnmatchedDstLeftAtZero(t *testing.T) { + c := fieldcopy.Copy[srcBasic, dstWithDefault]{} + c.Init() + src := srcBasic{Name: "alice", Age: 30, Email: "a@b.c"} + dst := c.Do(&src) + assert.Equal(t, "alice", dst.Name) + assert.Equal(t, 30, dst.Age) + assert.Equal(t, "", dst.Default) +} + +type srcRenamed struct { + FullName string + Age int +} + +type dstRenamed struct { + Name string + Age int +} + +func TestDoRename(t *testing.T) { + c := fieldcopy.Copy[srcRenamed, dstRenamed]{ + Rename: map[string]string{"Name": "FullName"}, + } + c.Init() + src := srcRenamed{FullName: "alice", Age: 30} + dst := c.Do(&src) + assert.Equal(t, "alice", dst.Name) + assert.Equal(t, 30, dst.Age) +} + +func TestReportAllMatched(t *testing.T) { + c := fieldcopy.Copy[srcBasic, dstBasic]{} + c.Init() + report := c.Report() + assert.Contains(t, report, "all fields matched") +} + +func TestReportUnmatchedSrc(t *testing.T) { + c := fieldcopy.Copy[srcWithExtra, dstSmall]{} + c.Init() + report := c.Report() + assert.Contains(t, report, "src not copied:") + assert.Contains(t, report, "- Extra") +} + +func TestReportUnmatchedDst(t *testing.T) { + c := fieldcopy.Copy[srcBasic, dstWithDefault]{} + c.Init() + report := c.Report() + assert.Contains(t, report, "dst not set:") + assert.Contains(t, report, "- Default") +} + +func TestReportWithRename(t *testing.T) { + c := fieldcopy.Copy[srcRenamed, dstRenamed]{ + Rename: map[string]string{"Name": "FullName"}, + } + report := c.Report() + assert.Contains(t, report, "all fields matched") +} + +func TestReportStaleRename(t *testing.T) { + c := fieldcopy.Copy[srcRenamed, dstRenamed]{ + Rename: map[string]string{ + "Name": "FullName", + "NonExistent": "Age", + }, + } + report := c.Report() + assert.Contains(t, report, "stale renames:") + assert.Contains(t, report, "NonExistent") +} + +type srcTypeMismatch struct { + Name string + Age string // string instead of int +} + +func TestDoTypeMismatchFieldSkipped(t *testing.T) { + c := fieldcopy.Copy[srcTypeMismatch, dstBasic]{} + c.Init() + src := srcTypeMismatch{Name: "alice", Age: "30"} + dst := c.Do(&src) + assert.Equal(t, "alice", dst.Name) + assert.Equal(t, 0, dst.Age) // not copied due to type mismatch + assert.Equal(t, "", dst.Email) // no match on src +} + +func TestReportTypeMismatch(t *testing.T) { + c := fieldcopy.Copy[srcTypeMismatch, dstBasic]{} + c.Init() + report := c.Report() + // Age exists on both but types don't match, so it's unmatched on both sides. + assert.Contains(t, report, "src not copied:") + assert.Contains(t, report, "dst not set:") +} + +type srcPointer struct { + Name string + Items *[]string +} + +type dstPointer struct { + Name string + Items *[]string +} + +func TestDoPointerFields(t *testing.T) { + c := fieldcopy.Copy[srcPointer, dstPointer]{} + c.Init() + items := []string{"a", "b"} + src := srcPointer{Name: "alice", Items: &items} + dst := c.Do(&src) + assert.Equal(t, "alice", dst.Name) + require.NotNil(t, dst.Items) + assert.Equal(t, []string{"a", "b"}, *dst.Items) + // Pointer is shared (shallow copy). + assert.Same(t, src.Items, dst.Items) +} + +func TestDoNilPointerFields(t *testing.T) { + c := fieldcopy.Copy[srcPointer, dstPointer]{} + c.Init() + src := srcPointer{Name: "alice", Items: nil} + dst := c.Do(&src) + assert.Equal(t, "alice", dst.Name) + assert.Nil(t, dst.Items) +} + +type srcMap struct { + Tags map[string]string +} + +type dstMap struct { + Tags map[string]string +} + +func TestDoMapFields(t *testing.T) { + c := fieldcopy.Copy[srcMap, dstMap]{} + c.Init() + src := srcMap{Tags: map[string]string{"k": "v"}} + dst := c.Do(&src) + assert.Equal(t, map[string]string{"k": "v"}, dst.Tags) + // Map is shared (shallow copy). + src.Tags["k2"] = "v2" + assert.Equal(t, "v2", dst.Tags["k2"]) +} + +type srcSlice struct { + Items []string +} + +type dstSlice struct { + Items []string +} + +func TestDoSliceFields(t *testing.T) { + c := fieldcopy.Copy[srcSlice, dstSlice]{} + c.Init() + src := srcSlice{Items: []string{"a", "b"}} + dst := c.Do(&src) + assert.Equal(t, []string{"a", "b"}, dst.Items) +} + +func TestDoZeroValue(t *testing.T) { + c := fieldcopy.Copy[srcBasic, dstBasic]{} + c.Init() + src := srcBasic{} + dst := c.Do(&src) + assert.Equal(t, "", dst.Name) + assert.Equal(t, 0, dst.Age) + assert.Equal(t, "", dst.Email) +} + +type srcBool struct { + Enabled bool + Name string +} + +type dstBool struct { + Enabled bool + Name string +} + +func TestDoBoolZeroValue(t *testing.T) { + c := fieldcopy.Copy[srcBool, dstBool]{} + c.Init() + src := srcBool{Enabled: false, Name: "test"} + dst := c.Do(&src) + assert.Equal(t, false, dst.Enabled) + assert.Equal(t, "test", dst.Name) +} + +type srcNested struct { + Name string + Config *nestedConfig +} + +type dstNested struct { + Name string + Config *nestedConfig +} + +type nestedConfig struct { + Value int +} + +func TestDoNestedStructPointer(t *testing.T) { + c := fieldcopy.Copy[srcNested, dstNested]{} + c.Init() + src := srcNested{Name: "alice", Config: &nestedConfig{Value: 42}} + dst := c.Do(&src) + assert.Equal(t, "alice", dst.Name) + require.NotNil(t, dst.Config) + assert.Equal(t, 42, dst.Config.Value) +} + +// Verify that private (unexported) fields are ignored. +type srcPrivate struct { + Name string + private int //nolint:unused +} + +type dstPrivate struct { + Name string + private int //nolint:unused +} + +func TestDoIgnoresUnexportedFields(t *testing.T) { + c := fieldcopy.Copy[srcPrivate, dstPrivate]{} + c.Init() + dst := c.Do(&srcPrivate{Name: "test"}) + assert.Equal(t, "test", dst.Name) +} + +// ForceSendFields auto-handling tests. + +type srcFSF struct { + Name string + Age int + Extra string + ForceSendFields []string +} + +type dstFSF struct { + Name string + Age int + ForceSendFields []string +} + +func TestDoForceSendFieldsAutoFiltered(t *testing.T) { + c := fieldcopy.Copy[srcFSF, dstFSF]{} + c.Init() + src := srcFSF{ + Name: "alice", + Age: 30, + ForceSendFields: []string{"Name", "Age", "Extra"}, + } + dst := c.Do(&src) + assert.Equal(t, "alice", dst.Name) + assert.Equal(t, 30, dst.Age) + // "Extra" should be filtered out since it doesn't exist on dstFSF. + assert.Equal(t, []string{"Name", "Age"}, dst.ForceSendFields) +} + +func TestDoForceSendFieldsNil(t *testing.T) { + c := fieldcopy.Copy[srcFSF, dstFSF]{} + c.Init() + src := srcFSF{Name: "alice", ForceSendFields: nil} + dst := c.Do(&src) + assert.Equal(t, "alice", dst.Name) + assert.Nil(t, dst.ForceSendFields) +} + +func TestDoForceSendFieldsEmpty(t *testing.T) { + c := fieldcopy.Copy[srcFSF, dstFSF]{} + c.Init() + src := srcFSF{Name: "alice", ForceSendFields: []string{}} + dst := c.Do(&src) + assert.Nil(t, dst.ForceSendFields) +} + +func TestDoForceSendFieldsAllFiltered(t *testing.T) { + c := fieldcopy.Copy[srcFSF, dstFSF]{} + c.Init() + src := srcFSF{ForceSendFields: []string{"Extra", "NonExistent"}} + dst := c.Do(&src) + // All entries filtered out → nil. + assert.Nil(t, dst.ForceSendFields) +} + +// Embedded struct: ForceSendFields promoted from embedded type should NOT +// trigger auto-handling, since we only copy direct fields. + +type InnerFSF struct { + Name string + ForceSendFields []string +} + +type outerDstFSF struct { + InnerFSF + Extra string +} + +func TestDoForceSendFieldsNotAutoHandledWhenPromoted(t *testing.T) { + c := fieldcopy.Copy[srcFSF, outerDstFSF]{} + c.Init() + src := srcFSF{ + Name: "alice", + Extra: "x", + ForceSendFields: []string{"Name", "Extra"}, + } + dst := c.Do(&src) + assert.Equal(t, "x", dst.Extra) + // ForceSendFields is promoted from InnerFSF — autoFSF should not activate. + // The embedded InnerFSF is not copied (type mismatch), so ForceSendFields stays nil. + assert.Nil(t, dst.ForceSendFields) +} + +func TestReportEmbeddedStructShowsAsField(t *testing.T) { + c := fieldcopy.Copy[srcFSF, outerDstFSF]{} + c.Init() + report := c.Report() + // The embedded struct itself appears as unmatched dst field. + assert.Contains(t, report, "- InnerFSF") + // ForceSendFields on src is not auto-handled, so it shows as unmatched. + assert.Contains(t, report, "- ForceSendFields") +} + +func TestDoPanicsWithoutInit(t *testing.T) { + c := fieldcopy.Copy[srcBasic, dstBasic]{} + assert.PanicsWithValue(t, "fieldcopy: Do called on uninitialized Copy[fieldcopy_test.srcBasic, fieldcopy_test.dstBasic]; call Init first", func() { + c.Do(&srcBasic{}) + }) +} diff --git a/tools/update_golden.py b/tools/update_golden.py new file mode 100755 index 0000000000..f9a243d201 --- /dev/null +++ b/tools/update_golden.py @@ -0,0 +1,34 @@ +#!/usr/bin/env python3 +"""Run go test -update for all packages that use libs/testdiff.""" + +import os +import subprocess +import sys + +from pathlib import Path + + +def main(): + result = subprocess.run( + ["git", "grep", "-l", "libs/testdiff", "--", "**/*_test.go"], + capture_output=False, + stdout=subprocess.PIPE, + text=True, + check=True, + ) + + dirs = {str(Path(f).parent) for f in result.stdout.splitlines() if f} + dirs.discard("acceptance") + packages = sorted(f"./{d}" for d in dirs) + + if not packages: + print("No packages found.", file=sys.stderr) + sys.exit(1) + + cmd = ["go", "test", *packages, "-update"] + print(" ".join(cmd), file=sys.stderr, flush=True) + os.execvp(cmd[0], cmd) + + +if __name__ == "__main__": + main()