Diff ABT Experiments for real-time RC #11398
Conversation
Conflicts: FirebaseRemoteConfig/Sources/RCNConfigDBManager.m FirebaseRemoteConfig/Sources/RCNConfigExperiment.m FirebaseRemoteConfig/Tests/Unit/RCNConfigExperimentTest.m
Conflicts: FirebaseRemoteConfig/Sources/RCNConfigExperiment.m FirebaseRemoteConfig/Tests/Unit/RCNConfigExperimentTest.m
Generated by 🚫 Danger |
Coverage Report 1Affected Products
Test Logs |
|
|
||
| - (FIRRemoteConfigUpdate *)getConfigUpdateForNamespace:(NSString *)FIRNamespace | ||
| withExperimentChanges: | ||
| (NSMutableSet<NSString *> *)changedExperimentKeys { |
There was a problem hiding this comment.
instead of doing the diffing elsewhere and passing in the changed keys, could we not read the experiment metadata directly from the dbManager here and do the diffing in this method?
feels unnecessary to have to explicitly pass in the experiment-related keys to getConfigUpdateForNamespace, when the diffing logic can be self-contained within RCNConfigContent
There was a problem hiding this comment.
My thinking is that all experiment logic lives in RCNConfigExperiments including in-memory copies of active and fetched experiments so why not do the diffing where all of the experiments already are so that we don't need to load all of the data elsewhere to do the same thing. Plus where it's called in Fetch allows us to easily pass in the keys after getting the diffs from experiments
There was a problem hiding this comment.
But I could go either way
There was a problem hiding this comment.
I get that it works as-is, but you are having the Fetch class partially compute the results, then pass it to the Config class, just to have it passed back. Fetch doesn't/shouldn't need to know the details of how the ConfigUpdate is computed, otherwise in the future if we decide different metadata is needed (which will likely happen soon), fetch (or anywhere else we call this method) may need to be updated
|
Please close, merge, or comment. We plan to close stale PRs on November 28, 2023. |
Adds diffing logic between experiments fetched via real-time and experiments active on the device. The config keys that have been affected by the differences will be added to ConfigUpdate. Testing below:
Created experiments with https://docs.google.com/document/d/1MXxUg1JSBbh3m_Qy4tGPrGs3-Nh8FAwKmg5ZWOKQ3J8/edit
Add experiments to configs using Management API, example: https://rpc.corp.google.com/rpc/builder?rpcId=15352346049097272373
2.1) Add/Remove Experiment to Config Key.
2.2) Replace experiment in Config Key
2.3) Create new param with experiment attached