feat: Add cache usage option for identify calls #408
Conversation
| */ | ||
| public enum IdentifyCacheHandling { | ||
| /** | ||
| `keep` will maintain the current in memory store of any previously known flag values from the last identified context. |
There was a problem hiding this comment.
Is there a way to move the language away from talking about the memory store? Then this contract is more general and survives future changes we may make to the caching/memory store layers. It is also then in language that the customer would speak in before understanding the deeper structure of the SDK memory stores.
You used ‘maintain’ in the descriptions. I like maintain more than keep and I think choosing one verb is more clear.
I propose moving away from “onMiss” to avoid mixing positive and negative language.
I propose moving away from Discard to avoid implying losing data. If the consumer uses identify to go back to the previous context again, the flag variations may still be in the cache.
“MaintainUntilFresh” maintains the current flag variations until the SDK is able to fetch fresh flag variations for the provided context.
“MaintainOrUseCached” maintains the current flag variations until the SDK is able to use cached flag variations for the provided context or fetch fresh flag variations.
“DontMaintain” does not maintain the current flag variations. Immediately switches to cached flag variations for the provided context or defaults if no cached flag variations exist.
There was a problem hiding this comment.
I'm still unsure of this API being in terms of previous/current state and not in terms of desired state when init/start and the data store system Casey is working on are in terms of desired state.
There was a problem hiding this comment.
I think there's enough divergence of client/server-side SDKs (servers lacking identify) that I think we can't quite fit this into that spec. So my opinion is that we evaluate this change mostly independent of the data system stuff.
There was a problem hiding this comment.
Thought more about this. It may be a significant pro to keep this scoped to caching while we learn how people will use it. Offering a general contract too soon could be worse.
There was a problem hiding this comment.
Does this mean you don't want your initially proposed changes to the language?
I think there's enough divergence of client/server-side SDKs (servers lacking identify) that I think we can't quite fit this into that spec
☝🏼 I also think FDv2 for iOS is probably just going to also coincide with a v10 release where we can completely change these signatures. Hopeful it will be the whole re-write, but it will at least have to be a major version.
There was a problem hiding this comment.
From discussion this morning:
keep -> UseCache: No
keepOnMiss -> UseCache: OnlyIfAvailable
discard -> UseCache: Yes
keelerm84
changed the title
The cache handling option allows users to control the flag store transitions while identification network requests asynchronously resolve.
keelerm84
force-pushed
the
mk/sdk-767/discard-keep-identify
branch
from
688745f to
c8189d5
Compare
🤖 I have created a release *beep* *boop* --- ## [9.12.0](9.11.0...9.12.0) (2024-11-06) ### Features * Add `LDConfig.sendEvents` option to disable all events ([#414](#414)) ([9a51844](9a51844)) * Add cache usage option for identify calls ([#408](#408)) ([b928345](b928345)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: LaunchDarklyReleaseBot <[email protected]>
The cache handling option allows users to control the flag store transitions while identification network requests asynchronously resolve.