Skip to content

Commit

Permalink
Remove unnnecessary RetrieveEnd func, avoid using session (open-telem…
Browse files Browse the repository at this point in the history
…etry#888)

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
  • Loading branch information
Bogdan Drutu authored Oct 26, 2021
1 parent c1fe885 commit 4bd33fa
Show file tree
Hide file tree
Showing 10 changed files with 73 additions and 96 deletions.
4 changes: 0 additions & 4 deletions internal/configsource/envvarconfigsource/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,6 @@ func (e *envVarConfigSource) Retrieve(_ context.Context, selector string, params
return configprovider.NewRetrieved(defaultValue), nil
}

func (e *envVarConfigSource) RetrieveEnd(context.Context) error {
return nil
}

func (e *envVarConfigSource) Close(context.Context) error {
return nil
}
7 changes: 3 additions & 4 deletions internal/configsource/envvarconfigsource/source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,16 @@ func TestEnvVarConfigSource_Session(t *testing.T) {
defaults = make(map[string]interface{})
}

s := &envVarConfigSource{
source := &envVarConfigSource{
defaults: defaults,
}

ctx := context.Background()
defer func() {
assert.NoError(t, s.RetrieveEnd(ctx))
assert.NoError(t, s.Close(ctx))
assert.NoError(t, source.Close(ctx))
}()

r, err := s.Retrieve(ctx, tt.selector, config.NewMapFromStringMap(tt.params))
r, err := source.Retrieve(ctx, tt.selector, config.NewMapFromStringMap(tt.params))
if tt.wantErr != nil {
assert.Nil(t, r)
require.IsType(t, tt.wantErr, err)
Expand Down
6 changes: 1 addition & 5 deletions internal/configsource/etcd2configsource/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func newConfigSource(params configprovider.CreateParams, cfg *Config) (configsou
}, nil
}

func (s *etcd2ConfigSource) Retrieve(ctx context.Context, selector string, paramsConfigMap *config.Map) (configsource.Retrieved, error) {
func (s *etcd2ConfigSource) Retrieve(ctx context.Context, selector string, _ *config.Map) (configsource.Retrieved, error) {
resp, err := s.kapi.Get(ctx, selector, nil)
if err != nil {
return nil, err
Expand All @@ -73,10 +73,6 @@ func (s *etcd2ConfigSource) Retrieve(ctx context.Context, selector string, param
return configprovider.NewWatchableRetrieved(resp.Node.Value, s.newWatcher(watchCtx, selector, resp.Node.ModifiedIndex)), nil
}

func (s *etcd2ConfigSource) RetrieveEnd(context.Context) error {
return nil
}

func (s *etcd2ConfigSource) Close(context.Context) error {
for _, cancel := range s.closeFuncs {
cancel()
Expand Down
15 changes: 7 additions & 8 deletions internal/configsource/etcd2configsource/source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func TestSessionRetrieve(t *testing.T) {
},
}

session := &etcd2ConfigSource{logger: logger, kapi: kapi}
source := &etcd2ConfigSource{logger: logger, kapi: kapi}
testsCases := []struct {
params interface{}
expect *string
Expand All @@ -52,7 +52,7 @@ func TestSessionRetrieve(t *testing.T) {

for _, c := range testsCases {
t.Run(c.name, func(t *testing.T) {
retrieved, err := session.Retrieve(context.Background(), c.key, nil)
retrieved, err := source.Retrieve(context.Background(), c.key, nil)
if c.expect != nil {
assert.NoError(t, err)
_, okWatcher := retrieved.(configsource.Watchable)
Expand All @@ -61,8 +61,7 @@ func TestSessionRetrieve(t *testing.T) {
}
assert.Error(t, err)
assert.Nil(t, retrieved)
assert.NoError(t, session.RetrieveEnd(context.Background()))
assert.NoError(t, session.Close(context.Background()))
assert.NoError(t, source.Close(context.Background()))
})
}
}
Expand All @@ -78,7 +77,7 @@ func TestWatcher(t *testing.T) {
close bool
}{
{name: "updated", close: false, result: "v", err: nil},
{name: "session-closed", close: true, result: "", err: nil},
{name: "source-closed", close: true, result: "", err: nil},
{name: "client-error", close: false, result: "", err: errors.New("client error")},
}

Expand All @@ -87,8 +86,8 @@ func TestWatcher(t *testing.T) {
watcher := newMockWatcher()
kapi.activeWatcher = watcher

session := &etcd2ConfigSource{logger: logger, kapi: kapi}
retrieved, err := session.Retrieve(context.Background(), "k1", nil)
source := &etcd2ConfigSource{logger: logger, kapi: kapi}
retrieved, err := source.Retrieve(context.Background(), "k1", nil)
assert.NoError(t, err)
assert.NotNil(t, retrieved.Value)
retrievedWatcher, okWatcher := retrieved.(configsource.Watchable)
Expand All @@ -98,7 +97,7 @@ func TestWatcher(t *testing.T) {
go func() {
switch {
case c.close:
session.Close(context.Background())
source.Close(context.Background())
case c.err != nil:
watcher.errors <- c.err
case c.result != "":
Expand Down
4 changes: 0 additions & 4 deletions internal/configsource/includeconfigsource/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,6 @@ func (is *includeConfigSource) Retrieve(_ context.Context, selector string, para
return configprovider.NewWatchableRetrieved(buf.Bytes(), watchForUpdateFn), nil
}

func (is *includeConfigSource) RetrieveEnd(context.Context) error {
return nil
}

func (is *includeConfigSource) Close(context.Context) error {
if is.watcher != nil {
return is.watcher.Close()
Expand Down
16 changes: 8 additions & 8 deletions internal/configsource/includeconfigsource/source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,17 +59,17 @@ func TestIncludeConfigSource_Session(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s, err := newConfigSource(configprovider.CreateParams{}, &Config{})
source, err := newConfigSource(configprovider.CreateParams{}, &Config{})
require.NoError(t, err)
require.NotNil(t, s)
require.NotNil(t, source)

ctx := context.Background()
defer func() {
assert.NoError(t, s.Close(ctx))
assert.NoError(t, source.Close(ctx))
}()

file := path.Join("testdata", tt.selector)
r, err := s.Retrieve(ctx, file, config.NewMapFromStringMap(tt.params))
r, err := source.Retrieve(ctx, file, config.NewMapFromStringMap(tt.params))
if tt.wantErr != nil {
assert.Nil(t, r)
require.IsType(t, tt.wantErr, err)
Expand Down Expand Up @@ -121,13 +121,13 @@ func TestIncludeConfigSource_DeleteFileError(t *testing.T) {
t.Skip("Windows only test")
}

s, err := newConfigSource(configprovider.CreateParams{}, &Config{DeleteFiles: true})
source, err := newConfigSource(configprovider.CreateParams{}, &Config{DeleteFiles: true})
require.NoError(t, err)
require.NotNil(t, s)
require.NotNil(t, source)

ctx := context.Background()
defer func() {
assert.NoError(t, s.Close(ctx))
assert.NoError(t, source.Close(ctx))
}()

// Copy test file
Expand All @@ -143,7 +143,7 @@ func TestIncludeConfigSource_DeleteFileError(t *testing.T) {
assert.NoError(t, os.Remove(dst))
})

r, err := s.Retrieve(ctx, dst, nil)
r, err := source.Retrieve(ctx, dst, nil)
assert.IsType(t, &errFailedToDeleteFile{}, err)
assert.Nil(t, r)
}
4 changes: 0 additions & 4 deletions internal/configsource/vaultconfigsource/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,6 @@ func (v *vaultConfigSource) Retrieve(_ context.Context, selector string, _ *conf
return configprovider.NewWatchableRetrieved(value, watchForUpdateFn), nil
}

func (v *vaultConfigSource) RetrieveEnd(context.Context) error {
return nil
}

func (v *vaultConfigSource) Close(context.Context) error {
close(v.doneCh)

Expand Down
Loading

0 comments on commit 4bd33fa

Please sign in to comment.