From fec73db1116c8e346ca3d73429718481c707318e Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 26 Sep 2024 17:47:07 +0200 Subject: [PATCH] address comments --- bundle/config/loader/process_include.go | 24 ++++++-------------- bundle/config/loader/process_include_test.go | 21 ----------------- bundle/config/resources.go | 18 +++++++++++++++ bundle/config/resources_test.go | 16 +++++++++++++ 4 files changed, 41 insertions(+), 38 deletions(-) diff --git a/bundle/config/loader/process_include.go b/bundle/config/loader/process_include.go index 411279d71..1f8b61d6f 100644 --- a/bundle/config/loader/process_include.go +++ b/bundle/config/loader/process_include.go @@ -13,23 +13,12 @@ import ( "github.com/databricks/cli/libs/dyn" ) -var resourceTypes = []string{ - "job", - "pipeline", - "model", - "experiment", - "model_serving_endpoint", - "registered_model", - "quality_monitor", - "schema", - "cluster", -} - func validateFileFormat(configRoot dyn.Value, filePath string) diag.Diagnostics { - for _, typ := range resourceTypes { - for _, ext := range []string{fmt.Sprintf(".%s.yml", typ), fmt.Sprintf(".%s.yaml", typ)} { + for _, resourceDescription := range config.SupportedResources() { + singularName := resourceDescription.SingularName + for _, ext := range []string{fmt.Sprintf(".%s.yml", singularName), fmt.Sprintf(".%s.yaml", singularName)} { if strings.HasSuffix(filePath, ext) { - return validateSingleResourceDefined(configRoot, ext, typ) + return validateSingleResourceDefined(configRoot, ext, singularName) } } } @@ -46,6 +35,7 @@ func validateSingleResourceDefined(configRoot dyn.Value, ext, typ string) diag.D } resources := []resource{} + supportedResources := config.SupportedResources() // Gather all resources defined in the resources block. _, err := dyn.MapByPattern( @@ -55,7 +45,7 @@ func validateSingleResourceDefined(configRoot dyn.Value, ext, typ string) diag.D // The key for the resource. Eg: "my_job" for jobs.my_job. k := p[2].Key() // The type of the resource. Eg: "job" for jobs.my_job. - typ := strings.TrimSuffix(p[1].Key(), "s") + typ := supportedResources[p[1].Key()].SingularName resources = append(resources, resource{path: p, value: v, typ: typ, key: k}) return v, nil @@ -72,7 +62,7 @@ func validateSingleResourceDefined(configRoot dyn.Value, ext, typ string) diag.D // The key for the resource. Eg: "my_job" for jobs.my_job. k := p[4].Key() // The type of the resource. Eg: "job" for jobs.my_job. - typ := strings.TrimSuffix(p[3].Key(), "s") + typ := supportedResources[p[3].Key()].SingularName resources = append(resources, resource{path: p, value: v, typ: typ, key: k}) return v, nil diff --git a/bundle/config/loader/process_include_test.go b/bundle/config/loader/process_include_test.go index b9c84b6d4..dabe66fdc 100644 --- a/bundle/config/loader/process_include_test.go +++ b/bundle/config/loader/process_include_test.go @@ -3,8 +3,6 @@ package loader import ( "context" "path/filepath" - "reflect" - "strings" "testing" "github.com/databricks/cli/bundle" @@ -171,22 +169,3 @@ func TestProcessIncludeFormatFail(t *testing.T) { }) } } - -func TestResourceNames(t *testing.T) { - names := []string{} - typ := reflect.TypeOf(config.Resources{}) - for i := 0; i < typ.NumField(); i++ { - field := typ.Field(i) - jsonTags := strings.Split(field.Tag.Get("json"), ",") - singularName := strings.TrimSuffix(jsonTags[0], "s") - names = append(names, singularName) - } - - // Assert the contents of the two lists are equal. Please add the singular - // name of your resource to resourceNames global if you are adding a new - // resource. - assert.Equal(t, len(resourceTypes), len(names)) - for _, name := range names { - assert.Contains(t, resourceTypes, name) - } -} diff --git a/bundle/config/resources.go b/bundle/config/resources.go index a3afb7fc3..0f16ad40b 100644 --- a/bundle/config/resources.go +++ b/bundle/config/resources.go @@ -59,3 +59,21 @@ func (r *Resources) FindResourceByConfigKey(key string) (ConfigResource, error) return found[0], nil } + +type ResourceDescription struct { + SingularName string +} + +func SupportedResources() map[string]ResourceDescription { + return map[string]ResourceDescription{ + "jobs": {SingularName: "job"}, + "pipelines": {SingularName: "pipeline"}, + "models": {SingularName: "model"}, + "experiments": {SingularName: "experiment"}, + "model_serving_endpoints": {SingularName: "model_serving_endpoint"}, + "registered_models": {SingularName: "registered_model"}, + "quality_monitors": {SingularName: "quality_monitor"}, + "schemas": {SingularName: "schema"}, + "clusters": {SingularName: "cluster"}, + } +} diff --git a/bundle/config/resources_test.go b/bundle/config/resources_test.go index 6860d73da..c1b76118c 100644 --- a/bundle/config/resources_test.go +++ b/bundle/config/resources_test.go @@ -3,6 +3,7 @@ package config import ( "encoding/json" "reflect" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -61,3 +62,18 @@ func TestCustomMarshallerIsImplemented(t *testing.T) { }, "Resource %s does not have a custom unmarshaller", field.Name) } } + +func TestSupportedResources(t *testing.T) { + expected := map[string]ResourceDescription{} + typ := reflect.TypeOf(Resources{}) + for i := 0; i < typ.NumField(); i++ { + field := typ.Field(i) + jsonTags := strings.Split(field.Tag.Get("json"), ",") + singularName := strings.TrimSuffix(jsonTags[0], "s") + expected[jsonTags[0]] = ResourceDescription{SingularName: singularName} + } + + // Please add your resource to the SupportedResources() function in resources.go + // if you are adding a new resource. + assert.Equal(t, expected, SupportedResources()) +}