From 1db384018c5efc6c7b1a9a43d5f1268c97ddd58d Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 14 Nov 2024 18:39:38 +0100 Subject: [PATCH] Make `TableName` field part of quality monitor schema (#1903) ## Changes This field was special-cased in #1307 because it's not part of the JSON payload in the SDK struct. This approach, while pragmatic, meant it didn't show up in the JSON schema. While debugging an issue with quality monitors in #1900, I couldn't figure out why I was getting schema errors on this field, or how it was passed through to the TF representation. This commit removes the special case and makes it behave like everything else. ## Tests * Unit tests pass. * Confirmed that the updated schema failed validation before this change. --- bundle/config/mutator/initialize_urls_test.go | 5 ++--- .../config/mutator/process_target_mode_test.go | 17 ++++++++++++----- bundle/config/resources/quality_monitor.go | 16 +++++++--------- .../tfdyn/convert_quality_monitor_test.go | 2 +- .../schema/testdata/pass/quality_monitor.yml | 1 + bundle/schema/jsonschema.json | 4 ++++ libs/dyn/convert/struct_info.go | 9 --------- 7 files changed, 27 insertions(+), 27 deletions(-) diff --git a/bundle/config/mutator/initialize_urls_test.go b/bundle/config/mutator/initialize_urls_test.go index 16b67dac..ec4e790c 100644 --- a/bundle/config/mutator/initialize_urls_test.go +++ b/bundle/config/mutator/initialize_urls_test.go @@ -65,9 +65,8 @@ func TestInitializeURLs(t *testing.T) { }, QualityMonitors: map[string]*resources.QualityMonitor{ "qualityMonitor1": { - CreateMonitor: &catalog.CreateMonitor{ - TableName: "catalog.schema.qualityMonitor1", - }, + TableName: "catalog.schema.qualityMonitor1", + CreateMonitor: &catalog.CreateMonitor{}, }, }, Schemas: map[string]*resources.Schema{ diff --git a/bundle/config/mutator/process_target_mode_test.go b/bundle/config/mutator/process_target_mode_test.go index b694f627..4135d5fd 100644 --- a/bundle/config/mutator/process_target_mode_test.go +++ b/bundle/config/mutator/process_target_mode_test.go @@ -102,16 +102,23 @@ func mockBundle(mode config.Mode) *bundle.Bundle { "registeredmodel1": {CreateRegisteredModelRequest: &catalog.CreateRegisteredModelRequest{Name: "registeredmodel1"}}, }, QualityMonitors: map[string]*resources.QualityMonitor{ - "qualityMonitor1": {CreateMonitor: &catalog.CreateMonitor{TableName: "qualityMonitor1"}}, - "qualityMonitor2": { + "qualityMonitor1": { + TableName: "qualityMonitor1", CreateMonitor: &catalog.CreateMonitor{ - TableName: "qualityMonitor2", - Schedule: &catalog.MonitorCronSchedule{}, + OutputSchemaName: "catalog.schema", + }, + }, + "qualityMonitor2": { + TableName: "qualityMonitor2", + CreateMonitor: &catalog.CreateMonitor{ + OutputSchemaName: "catalog.schema", + Schedule: &catalog.MonitorCronSchedule{}, }, }, "qualityMonitor3": { + TableName: "qualityMonitor3", CreateMonitor: &catalog.CreateMonitor{ - TableName: "qualityMonitor3", + OutputSchemaName: "catalog.schema", Schedule: &catalog.MonitorCronSchedule{ PauseStatus: catalog.MonitorCronSchedulePauseStatusUnpaused, }, diff --git a/bundle/config/resources/quality_monitor.go b/bundle/config/resources/quality_monitor.go index 3c823e62..30ec4f91 100644 --- a/bundle/config/resources/quality_monitor.go +++ b/bundle/config/resources/quality_monitor.go @@ -13,17 +13,15 @@ import ( ) type QualityMonitor struct { - // Represents the Input Arguments for Terraform and will get - // converted to a HCL representation for CRUD - *catalog.CreateMonitor - - // This represents the id which is the full name of the monitor - // (catalog_name.schema_name.table_name) that can be used - // as a reference in other resources. This value is returned by terraform. - ID string `json:"id,omitempty" bundle:"readonly"` - + ID string `json:"id,omitempty" bundle:"readonly"` ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` URL string `json:"url,omitempty" bundle:"internal"` + + // The table name is a required field but not included as a JSON field in [catalog.CreateMonitor]. + TableName string `json:"table_name"` + + // This struct defines the creation payload for a monitor. + *catalog.CreateMonitor } func (s *QualityMonitor) UnmarshalJSON(b []byte) error { diff --git a/bundle/deploy/terraform/tfdyn/convert_quality_monitor_test.go b/bundle/deploy/terraform/tfdyn/convert_quality_monitor_test.go index 50bfce7a..f71abf43 100644 --- a/bundle/deploy/terraform/tfdyn/convert_quality_monitor_test.go +++ b/bundle/deploy/terraform/tfdyn/convert_quality_monitor_test.go @@ -15,8 +15,8 @@ import ( func TestConvertQualityMonitor(t *testing.T) { var src = resources.QualityMonitor{ + TableName: "test_table_name", CreateMonitor: &catalog.CreateMonitor{ - TableName: "test_table_name", AssetsDir: "assets_dir", OutputSchemaName: "output_schema_name", InferenceLog: &catalog.MonitorInferenceLog{ diff --git a/bundle/internal/schema/testdata/pass/quality_monitor.yml b/bundle/internal/schema/testdata/pass/quality_monitor.yml index a9be5932..79c4dd69 100644 --- a/bundle/internal/schema/testdata/pass/quality_monitor.yml +++ b/bundle/internal/schema/testdata/pass/quality_monitor.yml @@ -4,6 +4,7 @@ bundle: resources: quality_monitors: myqualitymonitor: + table_name: catalog.schema.quality_monitor inference_log: granularities: - a diff --git a/bundle/schema/jsonschema.json b/bundle/schema/jsonschema.json index dc0d7f95..703daafe 100644 --- a/bundle/schema/jsonschema.json +++ b/bundle/schema/jsonschema.json @@ -684,6 +684,9 @@ "description": "Configuration for monitoring snapshot tables.", "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/catalog.MonitorSnapshot" }, + "table_name": { + "$ref": "#/$defs/string" + }, "time_series": { "description": "Configuration for monitoring time series tables.", "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/catalog.MonitorTimeSeries" @@ -695,6 +698,7 @@ }, "additionalProperties": false, "required": [ + "table_name", "assets_dir", "output_schema_name" ] diff --git a/libs/dyn/convert/struct_info.go b/libs/dyn/convert/struct_info.go index 595e52ed..dc3ed4da 100644 --- a/libs/dyn/convert/struct_info.go +++ b/libs/dyn/convert/struct_info.go @@ -6,7 +6,6 @@ import ( "sync" "github.com/databricks/cli/libs/dyn" - "github.com/databricks/cli/libs/textutil" ) // structInfo holds the type information we need to efficiently @@ -85,14 +84,6 @@ func buildStructInfo(typ reflect.Type) structInfo { } name, _, _ := strings.Cut(sf.Tag.Get("json"), ",") - if typ.Name() == "QualityMonitor" && name == "-" { - urlName, _, _ := strings.Cut(sf.Tag.Get("url"), ",") - if urlName == "" || urlName == "-" { - name = textutil.CamelToSnakeCase(sf.Name) - } else { - name = urlName - } - } if name == "" || name == "-" { continue }