[WIP] [POC] Use config endpoint for workflows#3666
Conversation
Generated by 🚫 Danger |
vegaro
changed the base branch from
main
to
phase_4__v2_config_topic_handler_registry
vegaro
force-pushed
the
cesar/config-workflows-spike
branch
4 times, most recently
from
f6837f6 to
4472d84
Compare
vegaro
force-pushed
the
cesar/config-workflows-spike
branch
from
4472d84 to
8102036
Compare
tonidero
left a comment
tonidero
left a comment
There was a problem hiding this comment.
Nice!! Only gave it a quick look but this is pretty much what I was thinking indeed! Still a lot of things left to do, but it's nice to have the skeleton with this 🙌 cc @RevenueCat/coresdk would be great for you to take a look as well to see if you find any issues with this architecture!
|
|
||
| override fun clear() { | ||
| // No handler-owned state: the manager prunes this topic's persisted metadata when it leaves the | ||
| // active set, and blob retention drops any body it kept alive. |
There was a problem hiding this comment.
We will need to support clearing I think, so we clear blobs upon a user change (since some of this will be user-dependent (at least that was my thinking)
| fun workflowIdForOfferingId(offeringId: String): String? = | ||
| manager.topic(TOPIC_NAME) | ||
| ?.entries | ||
| ?.firstOrNull { (_, item) -> item.content.stringOrNull(KEY_OFFERING_IDENTIFIER) == offeringId } |
There was a problem hiding this comment.
Hmm I wonder if we should optimize this somehow... Maybe we could keep in memory a map of offering id to workflow id, so we could then access by id directly... but maybe I'm eagerly optimizing it... we should test with apps with a ton of offerings and workflows later on to make sure this works fine 🙏
| // WorkflowsConfigProvider. The manager is hoisted so the orchestrator can drive its lifecycle | ||
| // (foreground refresh + teardown). | ||
| var remoteConfigManager: RemoteConfigManager? = null | ||
| val workflowManager = if (appConfig.useWorkflows) { |
There was a problem hiding this comment.
We might want to rename the feature flag to something more generic, since it will mean more than just workflows I guess... But not a blocker.
| OfferingsConfigGate { appInBackground, appUserID, onReady -> | ||
| manager.awaitTopicsReady(listOf("workflows"), appInBackground, appUserID, onReady) | ||
| } | ||
| }, |
There was a problem hiding this comment.
Interesting mehanism... I believe it should work 👍
There was a problem hiding this comment.
right! I find the gate function very elegant haha
| suspend fun body(topicName: String, itemKey: String): ByteArray? { | ||
| awaitCurrentSync() | ||
| val item = topic(topicName)?.get(itemKey) ?: return null | ||
| val ref = item.blobRef ?: return item.content.toString().toByteArray() |
There was a problem hiding this comment.
Hmm I wonder if we want to just return back the item string in case blobRef is null.... I would consider this body to only return in case there is an actual blob associated. For the "metadata" part of the config, that probably needs to be fetched from the RemoteConfigTopicStore?
| */ | ||
| internal fun interface RemoteConfigBlobFetcher { | ||
| /** Ensures [ref] is cached, returning whether it is now on disk. The default wiring is a no-op (`false`). */ | ||
| suspend fun ensureDownloaded(ref: String): Boolean |
There was a problem hiding this comment.
We will also need to add some prioritization system, so when we call this, the corresponding blob gets downloaded ASAP, taking priority of any in progress.
3 participants
No description provided.