Consolidate bundle configuration loader function (#918)

## Changes

There were two functions related to loading a bundle configuration file;
one as a package function and one as a member function on the
configuration type. Loading the same configuration object twice doesn't
make sense and therefore we can consolidate to only using the package
function.

The package function would scan for known file names if the specified
path was a directory. This functionality was not in use because the
top-level bundle loader figures out the filename itself as of #580.

## Tests

Pass.
This commit is contained in:
Pieter Noordhuis 2023-10-25 14:55:56 +02:00 committed by GitHub
parent a5815a0b47
commit 6e21ced54a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 24 additions and 47 deletions

View File

@ -76,10 +76,11 @@ func Load(ctx context.Context, path string) (*Bundle, error) {
return nil, err
}
log.Debugf(ctx, "Loading bundle configuration from: %s", configFile)
err = bundle.Config.Load(configFile)
root, err := config.Load(configFile)
if err != nil {
return nil, err
}
bundle.Config = *root
return bundle, nil
}

View File

@ -58,27 +58,33 @@ type Root struct {
Experimental *Experimental `json:"experimental,omitempty"`
}
// Load loads the bundle configuration file at the specified path.
func Load(path string) (*Root, error) {
var r Root
stat, err := os.Stat(path)
raw, err := os.ReadFile(path)
if err != nil {
return nil, err
}
// If we were given a directory, assume this is the bundle root.
if stat.IsDir() {
path, err = FileNames.FindInPath(path)
if err != nil {
return nil, err
}
var r Root
err = yaml.Unmarshal(raw, &r)
if err != nil {
return nil, fmt.Errorf("failed to load %s: %w", path, err)
}
if err := r.Load(path); err != nil {
return nil, err
if r.Environments != nil && r.Targets != nil {
return nil, fmt.Errorf("both 'environments' and 'targets' are specified, only 'targets' should be used: %s", path)
}
return &r, nil
if r.Environments != nil {
//TODO: add a command line notice that this is a deprecated option.
r.Targets = r.Environments
}
r.Path = filepath.Dir(path)
r.SetConfigFilePath(path)
_, err = r.Resources.VerifyUniqueResourceIdentifiers()
return &r, err
}
// SetConfigFilePath configures the path that its configuration
@ -127,32 +133,6 @@ func (r *Root) InitializeVariables(vars []string) error {
return nil
}
func (r *Root) Load(path string) error {
raw, err := os.ReadFile(path)
if err != nil {
return err
}
err = yaml.Unmarshal(raw, r)
if err != nil {
return fmt.Errorf("failed to load %s: %w", path, err)
}
if r.Environments != nil && r.Targets != nil {
return fmt.Errorf("both 'environments' and 'targets' are specified, only 'targets' should be used: %s", path)
}
if r.Environments != nil {
//TODO: add a command line notice that this is a deprecated option.
r.Targets = r.Environments
}
r.Path = filepath.Dir(path)
r.SetConfigFilePath(path)
_, err = r.Resources.VerifyUniqueResourceIdentifiers()
return err
}
func (r *Root) Merge(other *Root) error {
err := r.Sync.Merge(r, other)
if err != nil {

View File

@ -25,8 +25,7 @@ func TestRootMarshalUnmarshal(t *testing.T) {
}
func TestRootLoad(t *testing.T) {
root := &Root{}
err := root.Load("../tests/basic/databricks.yml")
root, err := Load("../tests/basic/databricks.yml")
require.NoError(t, err)
assert.Equal(t, "basic", root.Bundle.Name)
}
@ -77,18 +76,15 @@ func TestRootMergeMap(t *testing.T) {
}
func TestDuplicateIdOnLoadReturnsError(t *testing.T) {
root := &Root{}
err := root.Load("./testdata/duplicate_resource_names_in_root/databricks.yml")
_, err := Load("./testdata/duplicate_resource_names_in_root/databricks.yml")
assert.ErrorContains(t, err, "multiple resources named foo (job at ./testdata/duplicate_resource_names_in_root/databricks.yml, pipeline at ./testdata/duplicate_resource_names_in_root/databricks.yml)")
}
func TestDuplicateIdOnMergeReturnsError(t *testing.T) {
root := &Root{}
err := root.Load("./testdata/duplicate_resource_name_in_subconfiguration/databricks.yml")
root, err := Load("./testdata/duplicate_resource_name_in_subconfiguration/databricks.yml")
require.NoError(t, err)
other := &Root{}
err = other.Load("./testdata/duplicate_resource_name_in_subconfiguration/resources.yml")
other, err := Load("./testdata/duplicate_resource_name_in_subconfiguration/resources.yml")
require.NoError(t, err)
err = root.Merge(other)