[FSSDK-11177] Decision Service CMAB + Optimizely Client + Impression event adjustment #394
Conversation
OptimizelySDK/Bucketing/Bucketer.cs
Outdated
| /// <param name="userId">User identifier</param> | ||
| /// <param name="trafficAllocations">Traffic allocations to use for bucketing</param> | ||
| /// <returns>Entity ID (string) if user is bucketed, null otherwise</returns> | ||
| public virtual Result<string> BucketToEntityId(ProjectConfig config, Experiment experiment, |
There was a problem hiding this comment.
We can accept ExperimentCore parameter, and reuse the part upto BucketToEntityId for the Bucket metheod below. Logic for bucket would be:
- get entityId by calling BucketToEntityId()
- get the variation from the config and return using the entityId from step 1
|
|
||
| if (config.CustomCache != null) | ||
| { | ||
| _cmabCache = config.CustomCache as LruCache<CmabCacheEntry>; |
There was a problem hiding this comment.
we should not assume that customCache must be LruCache. It could be any implementation
There was a problem hiding this comment.
the class field _cmabCache should also be of type ICache<>, not LruCache
| /// <param name="cmabConfig">Configuration for CMAB cache. If null, default values are used.</param> | ||
| /// <param name="cmabClient">Client for fetching decisions from the CMAB prediction service. If null, a default client is created.</param> | ||
| /// <param name="logger">Optional logger for recording service operations.</param> | ||
| public DefaultCmabService(CmabConfig cmabConfig = null, |
There was a problem hiding this comment.
instead of taking in the config here, we can take in a ICache<> here and push the resposibility of supplying the cache to the caller, similar to cmabClient
There was a problem hiding this comment.
I see all of them can be null. We can just make them required and put the construction responsibility to the caller
Summary
Test Plan
Issues