Thread FlagDetails.metadata into ProviderEvaluation flagMetadata#23
Thread FlagDetails.metadata into ProviderEvaluation flagMetadata#23typotter wants to merge 5 commits into
Conversation
- Add toOpenFeatureFlagMetadata() helper on [String: Any] that converts to [String: FlagMetadataValue], widening Swift Int to Int64 for FlagMetadataValue.of() compatibility - Replace flagMetadata: [:] with details.metadata.toOpenFeatureFlagMetadata() in all five ProviderEvaluation extensions - Point Package.swift at local dd-sdk-ios path to pick up metadata field - Add ProviderEvaluationMetadataTests covering populated and empty metadata
Uses branch reference while dd-sdk-ios PR #2989 is in review. Update to a version tag once that PR merges and a release is cut.
typotter
requested review from
dd-oleksii and
greghuels
and removed request for
a team
🐑 PR Shepherd is maintaining this PRI watch your PR and automatically fix CI failures, rebase your branch, handle flaky tests, and push it to the merge queue when it's ready. More about what I do → Guide To pause me on this PR, add the |
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f237aef946
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| dependencies: [ | ||
| .package(url: "https://github.com/open-feature/swift-sdk.git", "0.3.0"..<"0.4.0"), | ||
| .package(url: "https://github.com/DataDog/dd-sdk-ios.git", from: "3.2.0"), | ||
| .package(url: "https://github.com/DataDog/dd-sdk-ios.git", branch: "typo/thread-allocation-key-to-flag-details-metadata"), |
There was a problem hiding this comment.
This branch: dependency makes any tagged release of this provider unusable for normal SPM consumers: SwiftPM rejects packages with branch-based transitive dependencies when the root package depends on them by version (the documented install path is .upToNextMajor(from: "0.1.0")). In that scenario the resolver fails before building, so this metadata change should wait for a released dd-sdk-ios version (and matching CocoaPods constraint) instead of pinning the provider manifest to a feature branch.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Please remember to change this back before merging
|
Depends on: DataDog/dd-sdk-ios#2989 — adds |
|
FFL-2510 |
| platforms: [ | ||
| .iOS(.v14), | ||
| .macOS(.v12), | ||
| .macOS("12.6"), |
There was a problem hiding this comment.
Is this version bump related to the metadata change?
There was a problem hiding this comment.
the latest dd-sdk-ios package.swift has bumped to 12.6 so too must we here
| dependencies: [ | ||
| .package(url: "https://github.com/open-feature/swift-sdk.git", "0.3.0"..<"0.4.0"), | ||
| .package(url: "https://github.com/DataDog/dd-sdk-ios.git", from: "3.2.0"), | ||
| .package(url: "https://github.com/DataDog/dd-sdk-ios.git", branch: "typo/thread-allocation-key-to-flag-details-metadata"), |
There was a problem hiding this comment.
Please remember to change this back before merging
| value: true, | ||
| variant: "on", | ||
| reason: "targeting_match", | ||
| metadata: ["allocationKey": .string("alloc-abc"), "version": .string("2")] |
There was a problem hiding this comment.
Would add a few more types in here for more test coverage: int, double, bool, etc
| // MARK: - Private Helpers | ||
|
|
||
| private extension [String: AnyValue] { | ||
| /// Converts an AnyValue metadata dictionary to the OpenFeature FlagMetadataValue map. |
There was a problem hiding this comment.
Nice, looks like you were already expecting AnyValue on this end
- Add intMetadataThreadedIntoFlagMetadata test for .int -> FlagMetadataValue.integer - Add doubleMetadataThreadedIntoFlagMetadata test for .double -> FlagMetadataValue.double - Add boolMetadataThreadedIntoFlagMetadata test for .bool -> FlagMetadataValue.boolean
typotter
force-pushed
the
typo/thread-flag-details-metadata-to-openfeature
branch
from
87a80bc to
841d6b2
Compare
sameerank
left a comment
sameerank
left a comment
There was a problem hiding this comment.
I assume the smoke test depends on DataDog/dd-sdk-ios#2989 so you might have to wait a little before this CI passes and can be merged
2 participants
What and why?
FFL-2510 —
ProviderEvaluation+DatadogFlags.swiftwas passingflagMetadata: [:]for all flag types, discarding themetadatafield onFlagDetails(which carriesallocationKeyandextraLoggingprimitives from the Precomputed Flags response).How?
toOpenFeatureFlagMetadata()private extension on[String: AnyValue]that converts eachAnyValuecase (.string,.int,.double,.bool) to the correspondingFlagMetadataValueviaFlagMetadataValue.of(). Complex types (.dictionary,.array,.null) are dropped — not representable asFlagMetadataValue.ProviderEvaluationextensions now passdetails.metadata.toOpenFeatureFlagMetadata()instead of[:]ProviderEvaluationMetadataTestscovering String/Int/Double/Bool metadata propagation and empty-metadata pass-through (44/44 pass)Dependencies and merge order
Depends on: DataDog/dd-sdk-ios#2989 (adds
FlagDetails.metadata)Package.swiftcurrently pinsdd-sdk-iosto branchtypo/thread-allocation-key-to-flag-details-metadataso this PR can be reviewed. Before merging, update the pin to the released version tag once dd-sdk-ios #2989 merges and a release is cut. ThePackage.resolvedmust also be updated at that time.Verification
CI: Platform Tests (iOS/macOS/tvOS) ✓, Unit Tests ✓, Lint ✓.
Smoke Tests (iOS)fails because the smoke test project resolves dd-sdk-ios from GitHub and the feature branch isn't in its resolution path — this is a known branch-pin artifact that resolves when dd-sdk-ios #2989 merges.