feat: add provider status to client (#205)#237
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a getProviderStatus method to the Client interface and its implementation in OpenFeatureClient, updating the public API and adding unit tests for various provider states. The review feedback suggests refactoring this method into a Kotlin property named providerStatus to follow idiomatic Kotlin conventions and ensure consistency with the existing statusFlow property.
Signed-off-by: Marcin Stepien <marcin.stepien@fluxon.com>
mstepien
force-pushed
the
205-add-provider-status-to-client
branch
from
7680474 to
96568be
Compare
|
Looks good and spec-compliant. Nothing to add from my side. |
typotter
left a comment
typotter
left a comment
There was a problem hiding this comment.
Review findings
Missing test coverage (no existing line to anchor these to):
Fatal status untested — getProviderStatus() is never tested for OpenFeatureStatus.Fatal. BrokenInitProvider throws ProviderNotReadyError (which maps to Error), not ProviderFatalError, so the Fatal branch is never exercised through the new accessor. Add a FatalInitProvider helper analogous to BrokenInitProvider that throws ProviderFatalError, call setProviderAndWait, and assert client.getProviderStatus() is OpenFeatureStatus.Fatal.
NotReady status untested — No test confirms that getProviderStatus() returns NotReady before any provider is set. Add a test that calls OpenFeatureAPI.getClient() without first calling setProvider/setProviderAndWait and asserts client.getProviderStatus() == OpenFeatureStatus.NotReady.
Stale status untested — OpenFeatureStatus.Stale is reachable via ProviderStale emission but is never exercised through the new accessor. OverlyEmittingProvider is already in helpers — trigger a ProviderStale emission and assert client.getProviderStatus() == OpenFeatureStatus.Stale.
CHANGELOG — No entry for the new getProviderStatus() public API surface. Add under the next unreleased section:
* Add `getProviderStatus()` to `Client` interface to expose provider status synchronously (spec 1.7.1)
Signed-off-by: Marcin Stepien <marcin.stepien@fluxon.com>
Signed-off-by: Marcin Stepien <marcin.stepien@fluxon.com>
Signed-off-by: Marcin Stepien <marcin.stepien@fluxon.com>
|
Consider adding a test that calls |
|
The Consider adding a test using a provider whose |
Signed-off-by: Marcin Stepien <marcin.stepien@fluxon.com>
Signed-off-by: Marcin Stepien <marcin.stepien@fluxon.com>
Signed-off-by: Marcin Stepien <marcin.stepien@fluxon.com>
mstepien
force-pushed
the
205-add-provider-status-to-client
branch
from
2c30735 to
c82eef6
Compare
Has been covered by |
| if (providerStatus == OpenFeatureStatus.NotReady) { | ||
| throw OpenFeatureError.ProviderNotReadyError() | ||
| } else if (providerStatus is OpenFeatureStatus.Fatal) { | ||
| throw OpenFeatureError.ProviderFatalError() |
There was a problem hiding this comment.
Not your code, but while you're here, we should propagate the underlying error
| throw (providerStatus as OpenFeatureStatus.Fatal).error |
Updated string keys for telemetry events to align with the OpenFeature specification Appendix D and OpenTelemetry Semantic Convention Release v1.33.0 and v1.34.0. Signed-off-by: Marcin Stepien <marcin.stepien@fluxon.com>
Updated test coverage. Updated Telemetry.kt to omit missing provider name and context ID entirely from the OpenTelemetry payload instead of sending empty strings or null values. This aligns with OpenTelemetry best practices for handling absent data. Signed-off-by: Marcin Stepien <marcin.stepien@fluxon.com>
4 participants
Provider status at Client
Implements a synchronous provider status accessor across the core SDK to fully align with OpenFeature Specifications requirements 1.7.1 and 1.7.2.1.
getProviderStatus(): OpenFeatureStatusbinding to theClientinterface and NativeOpenFeatureClientimplementation.RECONCILINGstate under the static-context paradigm whenever global evaluation contexts are swapped, matching upstream specification requirements.Related Issues
Fixes #205
Notes
Opted to retain
fun getProviderStatus()over converting it to an idiomatic Kotlin property block inside the interface.How to test
Run the test block directly in your CLI using:
./gradlew :kotlin-sdk:testDebugUnitTest --tests "dev.openfeature.kotlin.sdk.OpenFeatureClientTests"