Skip to content

Commit

Permalink
do not use atomic
Browse files Browse the repository at this point in the history
  • Loading branch information
ktong committed Feb 10, 2024
1 parent 23d42a3 commit 79b5829
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 23 deletions.
37 changes: 17 additions & 20 deletions provider/appconfig/appconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"log/slog"
"reflect"
"sync"
"sync/atomic"
"time"

"github.com/aws/aws-sdk-go-v2/aws"
Expand All @@ -39,11 +38,11 @@ type AppConfig struct {
pollInterval time.Duration

timeout time.Duration
token atomic.Pointer[string]
token *string
}

// New creates an AppConfig with the given application, environment, profile and Option(s).
func New(application, environment, profile string, opts ...Option) *AppConfig {
func New(application, environment, profile string, opts ...Option) AppConfig {
if application == "" {
panic("cannot create AppConfig with empty application")
}
Expand Down Expand Up @@ -76,14 +75,14 @@ func New(application, environment, profile string, opts ...Option) *AppConfig {
}
option.client = &clientProxy{config: option.awsConfig}

return &option.AppConfig
return option.AppConfig
}

func (a *AppConfig) Load() (map[string]any, error) {
func (a AppConfig) Load() (map[string]any, error) {
ctx, cancel := context.WithTimeout(context.Background(), a.timeout)
defer cancel()

if a.token.Load() == nil {
if a.token == nil {
input := &appconfigdata.StartConfigurationSessionInput{
ApplicationIdentifier: aws.String(a.application),
ConfigurationProfileIdentifier: aws.String(a.profile),
Expand All @@ -94,20 +93,20 @@ func (a *AppConfig) Load() (map[string]any, error) {
if err != nil {
return nil, err
}
a.token.Store(output.InitialConfigurationToken)
a.token = output.InitialConfigurationToken
}

return a.load(ctx)
}

func (a *AppConfig) Watch(ctx context.Context, onChange func(map[string]any)) error {
func (a AppConfig) Watch(ctx context.Context, onChange func(map[string]any)) error {
ticker := time.NewTicker(a.pollInterval)
defer ticker.Stop()

for {
select {
case <-ticker.C:
out, err := a.load(ctx)
values, err := a.load(ctx)
if err != nil {
a.logger.WarnContext(
ctx, "Error when reloading from AWS AppConfig",
Expand All @@ -120,27 +119,27 @@ func (a *AppConfig) Watch(ctx context.Context, onChange func(map[string]any)) er
continue
}

if out != nil {
onChange(out)
if values != nil {
onChange(values)
}
case <-ctx.Done():
return nil
}
}
}

func (a *AppConfig) load(ctx context.Context) (map[string]any, error) {
func (a AppConfig) load(ctx context.Context) (map[string]any, error) {
ctx, cancel := context.WithTimeout(ctx, a.timeout)
defer cancel()

input := &appconfigdata.GetLatestConfigurationInput{
ConfigurationToken: a.token.Load(),
ConfigurationToken: a.token,
}
output, err := a.client.GetLatestConfiguration(ctx, input)
if err != nil {
return nil, err
}
a.token.Store(output.NextPollConfigurationToken)
a.token = output.NextPollConfigurationToken

if len(output.Configuration) == 0 {
// It may return empty configuration data
Expand All @@ -156,7 +155,7 @@ func (a *AppConfig) load(ctx context.Context) (map[string]any, error) {
return out, nil
}

func (a *AppConfig) String() string {
func (a AppConfig) String() string {
return "appConfig:" + a.application + "-" + a.environment + "-" + a.profile
}

Expand Down Expand Up @@ -208,16 +207,14 @@ func (c *clientProxy) loadClient(ctx context.Context) (*appconfigdata.Client, er
c.clientOnce.Do(func() {
if reflect.ValueOf(c.config).IsZero() {
if c.config, err = config.LoadDefaultConfig(ctx); err != nil {
err = fmt.Errorf("load default AWS config: %w", err)

Check warning on line 210 in provider/appconfig/appconfig.go

View check run for this annotation

Codecov / codecov/patch

provider/appconfig/appconfig.go#L210

Added line #L210 was not covered by tests

return
}
}

c.client = appconfigdata.NewFromConfig(c.config)
})

if err != nil {
return nil, fmt.Errorf("load client: %w", err)
}

return c.client, nil
return c.client, err
}
2 changes: 1 addition & 1 deletion provider/appconfig/benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
)

func BenchmarkNew(b *testing.B) {
var loader *appconfig.AppConfig
var loader appconfig.AppConfig
for i := 0; i < b.N; i++ {
loader = appconfig.New("app", "env", "profile")
}
Expand Down
2 changes: 1 addition & 1 deletion provider/flag/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func WithDelimiter(delimiter string) Option {
}
}

// WithNameSplitter provides the function used to split environment variable names into nested keys.
// WithNameSplitter provides the function used to split flag names into nested keys.
// If it returns an nil/[]string{}/[]string{""}, the variable will be ignored.
//
// For example, with the default splitter, an flag name like "parent.child.key"
Expand Down
2 changes: 1 addition & 1 deletion provider/pflag/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func WithDelimiter(delimiter string) Option {
}
}

// WithNameSplitter provides the function used to split environment variable names into nested keys.
// WithNameSplitter provides the function used to split flag names into nested keys.
// If it returns an nil/[]string{}/[]string{""}, the variable will be ignored.
//
// For example, with the default splitter, an flag name like "parent.child.key"
Expand Down

0 comments on commit 79b5829

Please sign in to comment.