search

open-feature / go-sdk

Go SDK for OpenFeature
https://openfeature.dev
Apache License 2.0
126 stars 30 forks source link

feat: blocking provider mutator #251

Closed Kavindu-Dodan closed 7 months ago

Kavindu-Dodan commented 7 months ago

This PR

The provider MAY define a status field/accessor which indicates the readiness of the provider, with possible values NOT_READY, READY, STALE, or ERROR.

Providers without this field can be assumed to be ready immediately.

Related Issues

Fixes #240

How to test

Tests were added to cover new APIs and old tests were updated to handle readiness emitted for non eventing providers

codecov[bot] commented 7 months ago

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (d2c1636) 81.61% compared to head (f71a91c) 81.42%. Report is 1 commits behind head on main.

:exclamation: Current head f71a91c differs from pull request most recent head bfd25a3. Consider uploading reports for the commit bfd25a3 to get more accurate results

Files Patch % Lines
openfeature/api.go 94.33% 2 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #251 +/- ## ========================================== - Coverage 81.61% 81.42% -0.19% ========================================== Files 10 10 Lines 1142 1163 +21 ========================================== + Hits 932 947 +15 - Misses 192 196 +4 - Partials 18 20 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Kavindu-Dodan commented 7 months ago

This seems good, code is clean. I think my only issue is that the set+wait function should return errors if the provider init fails, that's a key difference between the blocking and non-blocking setProvider functions.

Returning an error allows users to completely sidestep setting up error handers (though they can still do so optionally and we'd expect them to fire).

I think you should implement this and add a couple tests:

  • one that returns an error for an init after some delay
  • one that returns an error and also ensures error handlers are run.

Relavant PR language improvement (editorial, not significant): open-feature/spec#242

Done, addressed this concern in the latest commit. I added error return as well as test to validate error return as well as eventing