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 | 15 +++++++++++---- 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, 26 insertions(+), 26 deletions(-) diff --git a/bundle/config/mutator/initialize_urls_test.go b/bundle/config/mutator/initialize_urls_test.go index 16b67dac81..ec4e790c4b 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 b694f627aa..4135d5fdf2 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"}}, + "qualityMonitor1": { + TableName: "qualityMonitor1", + CreateMonitor: &catalog.CreateMonitor{ + OutputSchemaName: "catalog.schema", + }, + }, "qualityMonitor2": { + TableName: "qualityMonitor2", CreateMonitor: &catalog.CreateMonitor{ - TableName: "qualityMonitor2", - Schedule: &catalog.MonitorCronSchedule{}, + 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 3c823e625b..30ec4f918a 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 50bfce7a01..f71abf43cc 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 a9be593298..79c4dd69b6 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 dc0d7f953b..703daafebd 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 595e52edde..dc3ed4da40 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 }