-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Adopt latest OF SDK #184
Conversation
8005890
to
0ce146b
Compare
<!-- Please use this template for your pull request. --> <!-- Please use the sections that you need and delete other sections --> ## Spec v0.8 adherence - Created "ProviderStatus.swift" according to [Specification](https://github.com/open-feature/spec/blob/634f2b52f990232b0b1c54c38f422cbff2c507bc/specification/types.md#provider-status). - ProviderState is now maintained by the SDK, and not by the Provider implementations - Align "ProviderEvent.swift" with [Specification](https://github.com/open-feature/spec/blob/634f2b52f990232b0b1c54c38f422cbff2c507bc/specification/types.md#provider-events) - Basic events like `Ready`, `Error` or `Fatal` are emitted by the SDK and are not expected by the Provider implementations. - Makes `initialize` and `onContextChange` protocols throwing, allowing for the OpenFeatureAPI layer to detect cases where `Error` events should be emitted - Makes `initialize` and `onContextChange` protocols async for an easier Provider implementation (in most cases) ### Missing parts - From the [Specification](https://openfeature.dev/specification/sections/events#conditional-requirement-5342): `It's possible that the on context changed function is invoked simultaneously or in quick succession; in this case the SDK will only run the PROVIDER_CONTEXT_CHANGED handlers after all reentrant invocations have terminated, and the last to terminate was successful (terminated normally)` - This is not part of this PR: subsequent calls are serialized and executed in sequence, each returning from its execution even if more same functions are queued - Handling of [spontaneous Provider events](https://openfeature.dev/specification/sections/events#requirement-535) missing in this PR ## Changes unrelated to Spec v0.8 - Added `setEvaluationContextAndWait` to provide the same better ergonomics that `setProviderAndWait` already offers - Evaluation returns early if provider state is not "ready" - Refactor `afterAll` hook to adhere to latest conventions ## Notes on backwards compatibility - A backwards incompatible aspect of this change is that the `initialize()` implementation needs to throw in case of errors, rather than emitting the `ERROR` event. The latter is now responsibility of the SDK. This is also valid for `onContextChange`. - This change in semantics is not enforced with code, and it might be easy for the Provider implementer to make mistakes when adopting this new version of the SDK (e.g. forget to remove `ERROR` emission from the Provider side) ## Example Adoption The Confidence OpenFeature Provider adopting this version of the SDK is in the works here: spotify/confidence-sdk-swift#184 --------- Signed-off-by: Fabrizio Demaria <[email protected]> Co-authored-by: Michael Beemer <[email protected]>
8372fdf
to
c842ba5
Compare
c842ba5
to
30dca28
Compare
Signed-off-by: Fabrizio Demaria <[email protected]>
30dca28
to
46ddc4c
Compare
@@ -18,7 +18,7 @@ let package = Package( | |||
targets: ["Confidence"]) | |||
], | |||
dependencies: [ | |||
.package(url: "[email protected]:open-feature/swift-sdk.git", from: "0.1.0"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interestingly, builds from main started failing since the dependency is "from" and not "exact". Maybe ok to keep it like this, forces us to keep things up to date :)
Signed-off-by: Fabrizio Demaria <[email protected]>
Signed-off-by: Fabrizio Demaria <[email protected]>
Signed-off-by: Fabrizio Demaria <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
Events are removed, since now handled automatically by the underlying SDK. onContextSet is now awaiting, returning on success or throwing on failure, as expected by the new SDK version.