Fix segfault in bundle summary command (#1937)

## Changes
This PR introduces use of new `isNil` method. It allows to ensure we
filter out all improperly defined resources in `bundle summary` command.
This includes deleted resources or resources with incorrect
configuration such as only defining key of the resource and nothing
else.

Fixes #1919, #1913

## Tests
Added regression unit test case
This commit is contained in:
Andrew Nester 2024-11-28 13:27:24 +01:00 committed by GitHub
parent 6fc2093a22
commit 8053e9c4e4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 86 additions and 11 deletions

View File

@ -41,6 +41,9 @@ type ConfigResource interface {
// InitializeURL initializes the URL field of the resource. // InitializeURL initializes the URL field of the resource.
InitializeURL(baseURL url.URL) InitializeURL(baseURL url.URL)
// IsNil returns true if the resource is nil, for example, when it was removed from the bundle.
IsNil() bool
} }
// ResourceGroup represents a group of resources of the same type. // ResourceGroup represents a group of resources of the same type.
@ -57,6 +60,9 @@ func collectResourceMap[T ConfigResource](
) ResourceGroup { ) ResourceGroup {
resources := make(map[string]ConfigResource) resources := make(map[string]ConfigResource)
for key, resource := range input { for key, resource := range input {
if resource.IsNil() {
continue
}
resources[key] = resource resources[key] = resource
} }
return ResourceGroup{ return ResourceGroup{

View File

@ -56,3 +56,7 @@ func (s *Cluster) GetName() string {
func (s *Cluster) GetURL() string { func (s *Cluster) GetURL() string {
return s.URL return s.URL
} }
func (s *Cluster) IsNil() bool {
return s.ClusterSpec == nil
}

View File

@ -79,3 +79,7 @@ func (r *Dashboard) GetName() string {
func (r *Dashboard) GetURL() string { func (r *Dashboard) GetURL() string {
return r.URL return r.URL
} }
func (r *Dashboard) IsNil() bool {
return r.Dashboard == nil
}

View File

@ -63,3 +63,7 @@ func (j *Job) GetName() string {
func (j *Job) GetURL() string { func (j *Job) GetURL() string {
return j.URL return j.URL
} }
func (j *Job) IsNil() bool {
return j.JobSettings == nil
}

View File

@ -58,3 +58,7 @@ func (s *MlflowExperiment) GetName() string {
func (s *MlflowExperiment) GetURL() string { func (s *MlflowExperiment) GetURL() string {
return s.URL return s.URL
} }
func (s *MlflowExperiment) IsNil() bool {
return s.Experiment == nil
}

View File

@ -58,3 +58,7 @@ func (s *MlflowModel) GetName() string {
func (s *MlflowModel) GetURL() string { func (s *MlflowModel) GetURL() string {
return s.URL return s.URL
} }
func (s *MlflowModel) IsNil() bool {
return s.Model == nil
}

View File

@ -66,3 +66,7 @@ func (s *ModelServingEndpoint) GetName() string {
func (s *ModelServingEndpoint) GetURL() string { func (s *ModelServingEndpoint) GetURL() string {
return s.URL return s.URL
} }
func (s *ModelServingEndpoint) IsNil() bool {
return s.CreateServingEndpoint == nil
}

View File

@ -58,3 +58,7 @@ func (p *Pipeline) GetName() string {
func (s *Pipeline) GetURL() string { func (s *Pipeline) GetURL() string {
return s.URL return s.URL
} }
func (s *Pipeline) IsNil() bool {
return s.PipelineSpec == nil
}

View File

@ -62,3 +62,7 @@ func (s *QualityMonitor) GetName() string {
func (s *QualityMonitor) GetURL() string { func (s *QualityMonitor) GetURL() string {
return s.URL return s.URL
} }
func (s *QualityMonitor) IsNil() bool {
return s.CreateMonitor == nil
}

View File

@ -68,3 +68,7 @@ func (s *RegisteredModel) GetName() string {
func (s *RegisteredModel) GetURL() string { func (s *RegisteredModel) GetURL() string {
return s.URL return s.URL
} }
func (s *RegisteredModel) IsNil() bool {
return s.CreateRegisteredModelRequest == nil
}

View File

@ -56,3 +56,7 @@ func (s *Schema) UnmarshalJSON(b []byte) error {
func (s Schema) MarshalJSON() ([]byte, error) { func (s Schema) MarshalJSON() ([]byte, error) {
return marshal.Marshal(s) return marshal.Marshal(s)
} }
func (s *Schema) IsNil() bool {
return s.CreateSchema == nil
}

View File

@ -519,6 +519,10 @@ func TestRenderSummary(t *testing.T) {
URL: "https://url2", URL: "https://url2",
JobSettings: &jobs.JobSettings{Name: "job2-name"}, JobSettings: &jobs.JobSettings{Name: "job2-name"},
}, },
"job3": {
ID: "3",
URL: "https://url3", // This emulates deleted job
},
}, },
Pipelines: map[string]*resources.Pipeline{ Pipelines: map[string]*resources.Pipeline{
"pipeline2": { "pipeline2": {

View File

@ -6,6 +6,8 @@ import (
"github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config"
"github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/bundle/config/resources"
"github.com/databricks/databricks-sdk-go/service/jobs"
"github.com/databricks/databricks-sdk-go/service/pipelines"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
) )
@ -14,11 +16,17 @@ func TestCompletions_SkipDuplicates(t *testing.T) {
Config: config.Root{ Config: config.Root{
Resources: config.Resources{ Resources: config.Resources{
Jobs: map[string]*resources.Job{ Jobs: map[string]*resources.Job{
"foo": {}, "foo": {
"bar": {}, JobSettings: &jobs.JobSettings{},
},
"bar": {
JobSettings: &jobs.JobSettings{},
},
}, },
Pipelines: map[string]*resources.Pipeline{ Pipelines: map[string]*resources.Pipeline{
"foo": {}, "foo": {
PipelineSpec: &pipelines.PipelineSpec{},
},
}, },
}, },
}, },
@ -36,10 +44,14 @@ func TestCompletions_Filter(t *testing.T) {
Config: config.Root{ Config: config.Root{
Resources: config.Resources{ Resources: config.Resources{
Jobs: map[string]*resources.Job{ Jobs: map[string]*resources.Job{
"foo": {}, "foo": {
JobSettings: &jobs.JobSettings{},
},
}, },
Pipelines: map[string]*resources.Pipeline{ Pipelines: map[string]*resources.Pipeline{
"bar": {}, "bar": {
PipelineSpec: &pipelines.PipelineSpec{},
},
}, },
}, },
}, },

View File

@ -7,6 +7,7 @@ import (
"github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config"
"github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/bundle/config/resources"
"github.com/databricks/databricks-sdk-go/service/jobs" "github.com/databricks/databricks-sdk-go/service/jobs"
"github.com/databricks/databricks-sdk-go/service/pipelines"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
@ -28,8 +29,12 @@ func TestLookup_NotFound(t *testing.T) {
Config: config.Root{ Config: config.Root{
Resources: config.Resources{ Resources: config.Resources{
Jobs: map[string]*resources.Job{ Jobs: map[string]*resources.Job{
"foo": {}, "foo": {
"bar": {}, JobSettings: &jobs.JobSettings{},
},
"bar": {
JobSettings: &jobs.JobSettings{},
},
}, },
}, },
}, },
@ -45,10 +50,14 @@ func TestLookup_MultipleFound(t *testing.T) {
Config: config.Root{ Config: config.Root{
Resources: config.Resources{ Resources: config.Resources{
Jobs: map[string]*resources.Job{ Jobs: map[string]*resources.Job{
"foo": {}, "foo": {
JobSettings: &jobs.JobSettings{},
},
}, },
Pipelines: map[string]*resources.Pipeline{ Pipelines: map[string]*resources.Pipeline{
"foo": {}, "foo": {
PipelineSpec: &pipelines.PipelineSpec{},
},
}, },
}, },
}, },
@ -92,10 +101,14 @@ func TestLookup_NominalWithFilters(t *testing.T) {
Config: config.Root{ Config: config.Root{
Resources: config.Resources{ Resources: config.Resources{
Jobs: map[string]*resources.Job{ Jobs: map[string]*resources.Job{
"foo": {}, "foo": {
JobSettings: &jobs.JobSettings{},
},
}, },
Pipelines: map[string]*resources.Pipeline{ Pipelines: map[string]*resources.Pipeline{
"bar": {}, "bar": {
PipelineSpec: &pipelines.PipelineSpec{},
},
}, },
}, },
}, },