FFL-1460 FlagsClient changes to accommodate Flags RN SDK sync flag evals#3049
Conversation
…nd tracking evaluations
f3efb76 to
ca508fe
Compare
0xnm
left a comment
There was a problem hiding this comment.
I added some suggestions, nothing blocking, mostly about API/logging. I think it is a final round and we are good to go after.
| val errorMessage = "Failed to parse value '${flag.variationValue}' as JSONObject - ${exception.message}" | ||
| featureSdkCore.internalLogger.log( | ||
| InternalLogger.Level.WARN, | ||
| InternalLogger.Target.MAINTAINER, |
There was a problem hiding this comment.
Should the target be the USER? Since I guess it is about user data parsing
There was a problem hiding this comment.
This log is corresponding to the flags data parsing logic in readAndParseAssignment (line 289).
There was a problem hiding this comment.
My point here is that MAINTAINER target logs are visible only in the SDK debug builds, they won't be shown in the release build. So if user needs to know about this error, it should be USER target.
There was a problem hiding this comment.
@typotter FYI the target has been changed to USER in other related flag value parsing logic as well. Does that work for you?
|
🎯 Code Coverage 🔗 Commit SHA: 0046c64 | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3049 +/- ##
===========================================
+ Coverage 71.46% 71.55% +0.09%
===========================================
Files 880 881 +1
Lines 32480 32544 +64
Branches 5473 5481 +8
===========================================
+ Hits 23211 23286 +75
- Misses 7728 7739 +11
+ Partials 1541 1519 -22
🚀 New features to boost your workflow:
|
typotter
left a comment
There was a problem hiding this comment.
lgtm, approved.
Just a suggestion but nothing blocking from ffe. 🚢
| flag.variationValue | ||
| } | ||
| else -> null | ||
| val variationTypeToKClass = mapOf( |
There was a problem hiding this comment.
nit: this could be a static constant instead of being redefined everytime the function runs.
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
What does this PR do?
This PR introduces new exposed "internal" APIs supporting the upcoming Flags SDK for React Native.
Changes:
_FlagsInternalProxyclass forFlagsClientUnparsedFlaginterface to preventPrecomputedFlagfrom being exposed to the public"internal" APIs = exposed APIs that are added purely for consumption within the RN SDK. This implementation follows the example of
_RumInternalProxy.Motivation
The motivation for this work is to allow the upcoming Flags RN SDK to retrieve the complete feature flags state snapshot from the JS side at SDK init time (FFL-1460).
Additional Notes
I've been developing these changes using the example application in the dd-sdk-reactnative repo (by leveraging composite builds)
Sibling PRs in the React Native and iOS repos:
Review checklist (to be filled by reviewers)