[AC-154] Adds basic nimbus flag building to ads-client - #7551
[AC-154] Adds basic nimbus flag building to ads-client#7551thesuzerain wants to merge 3 commits into
Conversation
| #[derive(Clone, Hash, PartialEq, Eq)] | ||
| pub enum NimbusFlag { | ||
| // `ads-client.async-enabled` | ||
| AsyncEnabled, |
There was a problem hiding this comment.
This is not used in this PR, obviously, but it will be the first flag we use (and will be used imminently in the other async PR). Just wanted to have an actual example, but if we want to avoid that as it's not used in this PR yet, I can delete.
| } | ||
| let flags = Arc::new(inner.nimbus_flags.clone().unwrap_or_default()); | ||
| let inner = Mutex::new(client); | ||
| MozAdsClient { inner, flags } |
There was a problem hiding this comment.
I think this makes sense to include at the MozAdsClient level given that it's surface-passed flags, and they may be referenced at this level as well. (if we want an early branch in the FFI layer for example of which of two wholly different internal functions to call- eg: recordimpression sync vs fire-and-forget)
There was a problem hiding this comment.
I pictured that nimbus flags fully end at this layer actually and they are translated into whatever they actually mean do the client at the time we construct/call the client.
So in the async case this is something like not storing the NimbusFlag.AsyncEnabled, but just constructing either AsyncAdsClient or SyncAdsClient. If they were not as dramatic as needing separate implementations then they could be translated into an option to the AdsClient constructor options.
For other situations like per-request flags they would be translated into some request option and have no reason to ever be stored at all.
There was a problem hiding this comment.
nit: I think there is a confusion, MozAdsClient and AdsClient are the same. MozAdsClient is only the uniffi version of it that encapsulate surface API for better tracking of breacking changes but it should not hold any feature so flags should go to AdsClient struct.
3 participants
Per discussion: We don't currently have the capacity to easily check for nimbus flags in rust components at the moment (though we expect to eventually), but we want to use nimbus to experiment, so we create a small structure to allow the flags to be easily passed from the outer surfaces.
If the set of all flags (or at least all
ads-clientones) are passed from the surface, that allows us to read the current set of active experiments in much the same way the surfaces can.Pull Request checklist
[ci full]to the PR title.