Skip to content

Commit

Permalink
Revert "fix: return the config file not found error (#5972)" (#6100)
Browse files Browse the repository at this point in the history
This reverts commit d2ddfe2.
  • Loading branch information
eapolinario authored Dec 11, 2024
1 parent ab4290e commit 2a7d363
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 51 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/single-binary.yml
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,8 @@ jobs:
with:
python-version: "3.12"
- uses: unionai/[email protected]
with:
version: '0.9.2'
- name: Setup sandbox
run: |
mkdir -p ~/.flyte/sandbox
Expand Down
6 changes: 2 additions & 4 deletions flytestdlib/config/tests/accessor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -431,11 +431,9 @@ func TestAccessor_UpdateConfig(t *testing.T) {
key := strings.ToUpper("my-component.str3")
assert.NoError(t, os.Setenv(key, "Set From Env"))
defer func() { assert.NoError(t, os.Unsetenv(key)) }()
err = v.UpdateConfig(context.TODO())
assert.Error(t, err)
assert.EqualError(t, err, "Config File \"config\" Not Found in \"[]\"")
assert.NoError(t, v.UpdateConfig(context.TODO()))
r := reg.GetSection(MyComponentSectionKey).GetConfig().(*MyComponentConfig)
assert.Equal(t, "", r.StringValue3)
assert.Equal(t, "Set From Env", r.StringValue3)
})

t.Run(fmt.Sprintf("[%v] Change handler", provider(config.Options{}).ID()), func(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions flytestdlib/config/tests/config_cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func TestDiscoverCommand(t *testing.T) {
t.Run(fmt.Sprintf(testNameFormatter, provider(config.Options{}).ID(), "No config file"), func(t *testing.T) {
cmd := config.NewConfigCommand(provider)
output, err := executeCommand(cmd, config.CommandDiscover)
assert.Error(t, err)
assert.NoError(t, err)
assert.Contains(t, output, "Couldn't find a config file.")
})

Expand All @@ -57,7 +57,7 @@ func TestValidateCommand(t *testing.T) {
t.Run(fmt.Sprintf(testNameFormatter, provider(config.Options{}).ID(), "No config file"), func(t *testing.T) {
cmd := config.NewConfigCommand(provider)
output, err := executeCommand(cmd, config.CommandValidate)
assert.Error(t, err)
assert.NoError(t, err)
assert.Contains(t, output, "Couldn't find a config file.")
})

Expand Down
2 changes: 0 additions & 2 deletions flytestdlib/config/viper/testdata/viper_test_config.yaml

This file was deleted.

30 changes: 17 additions & 13 deletions flytestdlib/config/viper/viper.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,34 +128,38 @@ func (v viperAccessor) updateConfig(ctx context.Context, r config.Section) error

v.viper.AutomaticEnv() // read in environment variables that match

shouldWatchChanges := true
// If a config file is found, read it in.
if err = v.viper.ReadInConfig(); err == nil {
logger.Debugf(ctx, "Using config file: %+v", v.viper.ConfigFilesUsed())
} else if asErrorCollection, ok := err.(stdLibErrs.ErrorCollection); ok {
shouldWatchChanges = false
for i, e := range asErrorCollection {
if _, isNotFound := errors.Cause(e).(viperLib.ConfigFileNotFoundError); isNotFound {
logger.Errorf(ctx, "[%v] Couldn't find a config file [%v]. Relying on env vars and pflags.",
logger.Infof(ctx, "[%v] Couldn't find a config file [%v]. Relying on env vars and pflags.",
i, v.viper.underlying[i].ConfigFileUsed())
return e
} else {
return err
}
}
return err
} else if reflect.TypeOf(err) == reflect.TypeOf(viperLib.ConfigFileNotFoundError{}) {
logger.Errorf(ctx, "Couldn't find a config file. Relying on env vars and pflags.")
return err
shouldWatchChanges = false
logger.Info(ctx, "Couldn't find a config file. Relying on env vars and pflags.")
} else {
return err
}

v.watcherInitializer.Do(func() {
// Watch config files to pick up on file changes without requiring a full application restart.
// This call must occur after *all* config paths have been added.
v.viper.OnConfigChange(func(e fsnotify.Event) {
logger.Debugf(ctx, "Got a notification change for file [%v] \n", e.Name)
v.configChangeHandler()
if shouldWatchChanges {
v.watcherInitializer.Do(func() {
// Watch config files to pick up on file changes without requiring a full application restart.
// This call must occur after *all* config paths have been added.
v.viper.OnConfigChange(func(e fsnotify.Event) {
logger.Debugf(ctx, "Got a notification change for file [%v] \n", e.Name)
v.configChangeHandler()
})
v.viper.WatchConfig()
})
v.viper.WatchConfig()
})
}

return v.RefreshFromConfig(ctx, r, true)
}
Expand Down
30 changes: 0 additions & 30 deletions flytestdlib/config/viper/viper_test.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
package viper

import (
"context"
"encoding/base64"
"reflect"
"testing"

"github.com/stretchr/testify/assert"

"github.com/flyteorg/flyte/flytestdlib/config"
)

func Test_stringToByteArray(t *testing.T) {
Expand Down Expand Up @@ -55,30 +52,3 @@ func Test_stringToByteArray(t *testing.T) {
assert.NotEqual(t, []byte("hello"), res)
})
}

func TestViperAccessor_UpdateConfig(t *testing.T) {
ctx := context.Background()
t.Run("unable to find the config file", func(t *testing.T) {
// Create accessor
accessor := newAccessor(config.Options{
SearchPaths: []string{".", "/etc/flyte/config", "$GOPATH/src/github.com/flyteorg/flyte"},
StrictMode: false,
})

// Update config
err := accessor.updateConfig(ctx, accessor.rootConfig)
assert.EqualError(t, err, "Config File \"config\" Not Found in \"[]\"")
})

t.Run("find the config file", func(t *testing.T) {
// Create accessor
accessor := newAccessor(config.Options{
SearchPaths: []string{"./testdata/viper_test_config.yaml"},
StrictMode: false,
})

// Update config
err := accessor.updateConfig(ctx, accessor.rootConfig)
assert.NoError(t, err)
})
}

0 comments on commit 2a7d363

Please sign in to comment.