8000 [go-sdk] Post bug bash optimizations - hide retryConfig (#419) · optimizely/go-sdk@9f4ba4f · GitHub
[go: up one dir, main page]
Skip to content

Commit 9f4ba4f

Browse files
Mat001Mat001
authored
[go-sdk] Post bug bash optimizations - hide retryConfig (#419)
* Hide retry config from public API and change default retry count to 1 - Remove RetryConfig field from public cmab.Config struct - Change DefaultMaxRetries from 3 to 1 for conservative first release - Use hardcoded internal retry config in ExperimentCmabService - Create client-level CmabConfig type for stable public API - Update all tests to use new configuration approach This keeps retry functionality internal while preventing API exposure that might need to change in future releases. * format * Fix retry config handling per reviewer feedback Keep existing code structure instead of hardcoding retry config. Since RetryConfig is no longer in public API, always use defaults but maintain the same if/else structure for consistency. * Revert internal changes per reviewer feedback - keep only factory changes
1 parent 1c7a4f3 commit 9f4ba4f

File tree

4 files changed

+34
-25
lines changed

4 files changed

+34
-25
lines changed

‎pkg/client/factory.go‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"errors"
2323
"time"
2424

25+
"github.com/optimizely/go-sdk/v2/pkg/cache"
2526
"github.com/optimizely/go-sdk/v2/pkg/cmab"
2627
"github.com/optimizely/go-sdk/v2/pkg/config"
2728
"github.com/optimizely/go-sdk/v2/pkg/decide"
@@ -37,6 +38,28 @@ import (
3738
"github.com/optimizely/go-sdk/v2/pkg/utils"
3839
)
3940

41+
// CmabConfig holds CMAB configuration options exposed at the client level.
42+
// This provides a stable public API while allowing internal cmab.Config to change.
43+
type CmabConfig struct {
44+
CacheSize int
45+
CacheTTL time.Duration
46+
HTTPTimeout time.Duration
47+
Cache cache.CacheWithRemove // Custom cache implementation (Redis, etc.)
48+
}
49+
50+
// toCmabConfig converts client-level CmabConfig to internal cmab.Config
51+
func (c *CmabConfig) toCmabConfig() *cmab.Config {
52+
if c == nil {
53+
return nil
54+
}
55+
return &cmab.Config{
56+
CacheSize: c.CacheSize,
57+
CacheTTL: c.CacheTTL,
58+
HTTPTimeout: c.HTTPTimeout,
59+
Cache: c.Cache,
60+
}
61+
}
62+
4063
// OptimizelyFactory is used to customize and construct an instance of the OptimizelyClient.
4164
type OptimizelyFactory struct {
4265
SDKKey string
@@ -54,7 +77,7 @@ type OptimizelyFactory struct {
5477
overrideStore decision.ExperimentOverrideStore
5578
userProfileService decision.UserProfileService
5679
notificationCenter notification.Center
57-
cmabConfig *cmab.Config
80+
cmabConfig *CmabConfig
5881

5982
// ODP
6083
segmentsCacheSize int
@@ -163,7 +186,7 @@ func (f *OptimizelyFactory) Client(clientOptions ...OptionFunc) (*OptimizelyClie
163186
}
164187
// Add CMAB config option if provided
165188
if f.cmabConfig != nil {
166-
experimentServiceOptions = append(experimentServiceOptions, decision.WithCmabConfig(f.cmabConfig))
189+
experimentServiceOptions = append(experimentServiceOptions, decision.WithCmabConfig(f.cmabConfig.toCmabConfig()))
167190
}
168191
compositeExperimentService := decision.NewCompositeExperimentService(f.SDKKey, experimentServiceOptions...)
169192
compositeService := decision.NewCompositeService(f.SDKKey, decision.WithCompositeExperimentService(compositeExperimentService))
@@ -327,7 +350,7 @@ func WithTracer(tracer tracing.Tracer) OptionFunc {
327350
}
328351

329352
// WithCmabConfig sets the CMAB configuration options
330-
func WithCmabConfig(cmabConfig *cmab.Config) OptionFunc {
353+
func WithCmabConfig(cmabConfig *CmabConfig) OptionFunc {
331354
return func(f *OptimizelyFactory) {
332355
f.cmabConfig = cmabConfig
333356
}

‎pkg/client/factory_test.go‎

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import (
2828
"github.com/stretchr/testify/mock"
2929

3030
"github.com/optimizely/go-sdk/v2/pkg/cache"
31-
"github.com/optimizely/go-sdk/v2/pkg/cmab"
3231
"github.com/optimizely/go-sdk/v2/pkg/config"
3332
"github.com/optimizely/go-sdk/v2/pkg/decide"
3433
"github.com/optimizely/go-sdk/v2/pkg/decision"
@@ -461,13 +460,10 @@ func TestStaticClientError(t *testing.T) {
461460

462461
func TestFactoryWithCmabConfig(t *testing.T) {
463462
factory := OptimizelyFactory{}
464-
cmabConfig := cmab.Config{
463+
cmabConfig := CmabConfig{
465464
CacheSize: 100,
466465
CacheTTL: time.Minute,
467466
HTTPTimeout: 30 * time.Second,
468-
RetryConfig: &cmab.RetryConfig{
469-
MaxRetries: 5,
470-
},
471467
}
472468

473469
// Test the option function
@@ -477,19 +473,14 @@ func TestFactoryWithCmabConfig(t *testing.T) {
477473
assert.Equal(t, 100, factory.cmabConfig.CacheSize)
478474
assert.Equal(t, time.Minute, factory.cmabConfig.CacheTTL)
479475
assert.Equal(t, 30*time.Second, factory.cmabConfig.HTTPTimeout)
480-
assert.NotNil(t, factory.cmabConfig.RetryConfig)
481-
assert.Equal(t, 5, factory.cmabConfig.RetryConfig.MaxRetries)
482476
}
483477

484478
func TestFactoryCmabConfigPassedToDecisionService(t *testing.T) {
485479
// Test that CMAB config is correctly passed to decision service when creating client
486-
cmabConfig := cmab.Config{
480+
cmabConfig := CmabConfig{
487481
CacheSize: 200,
488482
CacheTTL: 2 * time.Minute,
489483
HTTPTimeout: 20 * time.Second,
490-
RetryConfig: &cmab.RetryConfig{
491-
MaxRetries: 3,
492-
},
493484
}
494485

495486
factory := OptimizelyFactory{
@@ -501,7 +492,6 @@ func TestFactoryCmabConfigPassedToDecisionService(t *testing.T) {
501492
assert.Equal(t, &cmabConfig, factory.cmabConfig)
502493
assert.Equal(t, 200, factory.cmabConfig.CacheSize)
503494
assert.Equal(t, 2*time.Minute, factory.cmabConfig.CacheTTL)
504-
assert.NotNil(t, factory.cmabConfig.RetryConfig)
505495
}
506496

507497
func TestFactoryOptionFunctions(t *testing.T) {
@@ -512,19 +502,19 @@ func TestFactoryOptionFunctions(t *testing.T) {
512502
WithSegmentsCacheSize(100)(factory)
513503
WithSegmentsCacheTimeout(5 * time.Second)(factory)
514504
WithOdpDisabled(true)(factory)
515-
WithCmabConfig(&cmab.Config{CacheSize: 50})(factory)
505+
WithCmabConfig(&CmabConfig{CacheSize: 50})(factory)
516506

517507
// Verify options were set
518508
assert.Equal(t, "test_token", factory.DatafileAccessToken)
519509
assert.Equal(t, 100, factory.segmentsCacheSize)
520510
assert.Equal(t, 5*time.Second, factory.segmentsCacheTimeout)
521511
assert.True(t, factory.odpDisabled)
522-
assert.Equal(t, &cmab.Config{CacheSize: 50}, factory.cmabConfig)
512+
assert.Equal(t, &CmabConfig{CacheSize: 50}, factory.cmabConfig)
523513
}
524514

525515
func TestWithCmabConfigOption(t *testing.T) {
526516
factory := &OptimizelyFactory{}
527-
testConfig := cmab.Config{
517+
testConfig := CmabConfig{
528518
CacheSize: 200,
529519
CacheTTL: 2 * time.Minute,
530520
}
@@ -534,13 +524,10 @@ func TestWithCmabConfigOption(t *testing.T) {
534524

535525
func TestClientWithCmabConfig(t *testing.T) {
536526
// Test client creation with non-empty CMAB config (tests reflect.DeepEqual path)
537-
cmabConfig := cmab.Config{
527+
cmabConfig := CmabConfig{
538528
CacheSize: 200,
539529
CacheTTL: 5 * time.Minute,
540530
HTTPTimeout: 30 * time.Second,
541-
RetryConfig: &cmab.RetryConfig{
542-
MaxRetries: 5,
543-
},
544531
}
545532

546533
factory := OptimizelyFactory{
@@ -559,7 +546,7 @@ func TestClientWithCmabConfig(t *testing.T) {
559546

560547
func TestClientWithEmptyCmabConfig(t *testing.T) {
561548
// Test client creation with empty CMAB config (tests reflect.DeepEqual returns true)
562-
emptyCmabConfig := cmab.Config{}
549+
emptyCmabConfig := CmabConfig{}
563550

564551
factory := OptimizelyFactory{
565552
SDKKey: "test_sdk_key",

‎pkg/cmab/client.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ var CMABPredictionEndpoint = "https://prediction.cmab.optimizely.com/predict/%s"
3535

3636
const (
3737
// DefaultMaxRetries is the default number of retries for CMAB requests
38-
DefaultMaxRetries = 3
38+
DefaultMaxRetries = 1
3939
// DefaultInitialBackoff is the default initial backoff duration
4040
DefaultInitialBackoff = 100 * time.Millisecond
4141
// DefaultMaxBackoff is the default maximum backoff duration

‎pkg/cmab/service_test.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -845,7 +845,6 @@ func (s *CmabServiceTestSuite) TestGetDecisionApiError() {
845845
s.mockClient.AssertExpectations(s.T())
846846
}
847847

848-
849848
// TestLockStripingDistribution verifies that different user/rule combinations
850849
// use different locks to allow for better concurrency
851850
func TestLockStripingDistribution(t *testing.T) {

0 commit comments

Comments
 (0)
0