open-feature / python-sdk

Python SDK for OpenFeature
https://openfeature.dev
Apache License 2.0
48 stars 16 forks source link

feat: implement provider events #278

Closed federicobond closed 5 months ago

federicobond commented 6 months ago

This PR

Follow-up Tasks

Fixes: https://github.com/open-feature/python-sdk/pull/278

codecov[bot] commented 6 months ago

Codecov Report

Attention: Patch coverage is 98.16514% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 94.77%. Comparing base (04b4009) to head (d4420f3).

Files Patch % Lines
openfeature/_event_support.py 97.50% 1 Missing :warning:
openfeature/provider/provider.py 91.66% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #278 +/- ## ========================================== + Coverage 94.19% 94.77% +0.58% ========================================== Files 18 20 +2 Lines 551 651 +100 ========================================== + Hits 519 617 +98 - Misses 32 34 +2 ``` | [Flag](https://app.codecov.io/gh/open-feature/python-sdk/pull/278/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/open-feature/python-sdk/pull/278/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature) | `94.77% <98.16%> (+0.58%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature#carryforward-flags-in-the-pull-request-comment) to find out more.

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

toddbaert commented 6 months ago

I'll take a close look at this in the next couple days.

toddbaert commented 6 months ago

@federicobond I've opened https://github.com/open-feature/spec/pull/248

federicobond commented 6 months ago

This should be ready to review from a specification perspective, we might want to add some further tests maybe.

federicobond commented 6 months ago

From Requirement 5.1.1:

Providers cannot emit PROVIDER_CONTEXT_CHANGED or PROVIDER_RECONCILING event.

Should we add a guard to make sure this cannot happen?

federicobond commented 6 months ago

@matthewelwell I refactored the EventSupport implementation so that we are no longer using private module imports. It should be much cleaner now.

federicobond commented 5 months ago

Merged! 🎉 Thanks everyone for your helpful comments and suggestions.

beeme1mr commented 5 months ago

Amazing, nice job @federicobond!