Skip to content

Conversation

@Mat001
Copy link
Contributor

@Mat001 Mat001 commented Oct 31, 2025

  • expose cmab prediction endpoint for server side SDKs so customers can define their own proxy for testing.
  • it has been exposed via datafile manager, works similarly to datafile URL template, new URL is added to config, then config is called in optimizely instance

Jira ticket:
https://optimizely-ext.atlassian.net/browse/FSSDK-12012

@Mat001 Mat001 self-assigned this Oct 31, 2025
@pvcraven pvcraven requested a review from Copilot November 3, 2025 15:18
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for configurable CMAB prediction endpoints, allowing users to specify custom endpoint URLs instead of being hard-coded to the default Optimizely endpoint. This is useful for scenarios like proxying requests through custom infrastructure or using alternative prediction services.

Key changes:

  • Refactored the global mutable CMABPredictionEndpoint variable into an instance-level configuration field
  • Added PredictionEndpoint field to CMAB configuration structs throughout the codebase
  • Updated tests to use the new configuration approach instead of mutating global state

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
pkg/cmab/config.go Added DefaultPredictionEndpointTemplate constant and PredictionEndpoint field to Config struct
pkg/cmab/client.go Replaced global CMABPredictionEndpoint variable with instance field and DefaultPredictionEndpoint constant; updated ClientOptions to include PredictionEndpoint
pkg/cmab/client_test.go Refactored tests to use PredictionEndpoint option instead of mutating global state; added new tests for custom endpoint functionality
pkg/decision/experiment_cmab_service.go Added logic to handle PredictionEndpoint configuration with appropriate fallback to defaults
pkg/client/factory.go Added PredictionEndpoint field to public CmabConfig struct and its conversion logic
pkg/client/factory_test.go Added tests for PredictionEndpoint configuration at the client factory level
Comments suppressed due to low confidence (1)

pkg/cmab/config.go:60

  • The NewDefaultConfig function does not set the PredictionEndpoint field despite it being part of the Config struct. This means users calling this function won't get the default prediction endpoint. Add PredictionEndpoint: DefaultPredictionEndpoint to the returned Config.
// NewDefaultConfig creates a Config with default values
func NewDefaultConfig() Config {
	return Config{
		CacheSize:   DefaultCacheSize,
		CacheTTL:    DefaultCacheTTL,
		HTTPTimeout: DefaultHTTPTimeout,
		RetryConfig: &RetryConfig{
			MaxRetries: DefaultMaxRetries,
		},
	}
}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

@pvcraven pvcraven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adds CMAB endpoint.

- Renamed field from PredictionEndpoint to PredictionEndpointTemplate for clarity
- Updated all references across client, config, factory, and tests
- Removed redundant constant definition from config.go
- Removed setting predictionEndpoint in NewExperimentCmabService when config is nil
- Changed test endpoint from endpoint.com to example.com (avoid real domain)
- All tests passing
@Mat001 Mat001 requested a review from raju-opti November 3, 2025 19:26
@Mat001 Mat001 merged commit d5236fb into master Nov 3, 2025
33 of 37 checks passed
@Mat001 Mat001 deleted the mpirnovar-expose-prediction-endpoint-fssdk-12012 branch November 3, 2025 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants