Skip to content

Conversation

@federicobond
Copy link
Member

@federicobond federicobond marked this pull request as ready for review February 7, 2024 15:16
@federicobond federicobond requested a review from a team as a code owner February 7, 2024 15:16
@codecov
Copy link

codecov bot commented Feb 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d8e10c7) 90.55% compared to head (4abf187) 89.92%.
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #37      +/-   ##
==========================================
- Coverage   90.55%   89.92%   -0.64%     
==========================================
  Files           8        6       -2     
  Lines         180      129      -51     
==========================================
- Hits          163      116      -47     
+ Misses         17       13       -4     
Flag Coverage Δ
unittests 89.92% <ø> (-0.64%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@matthewelwell matthewelwell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@federicobond the change here looks sound to me but I'm confused by the workflows failure. I can't seen anything in this PR that would cause this but it seems that the same workflow passed on the last run on the main branch and your last PR branch. Any ideas?

@gruebel
Copy link
Member

gruebel commented Feb 9, 2024

@matthewelwell is is probably a side-effect of the recently released version 0.4.2 with this change open-feature/python-sdk#268. The AbstractProvider class defines get_provider_hooks() as an abstract method with a default implementation, which somehow worked before, but now not anymore, which makes more sense for me

https://github.com/open-feature/python-sdk/blob/f9833ba75390008f1bb3c07149dc41be714a1f78/openfeature/provider/provider.py#L22-L24

either we add this method to the FlagdProvider class or we remove the @abstractmethod from the abstract class.

@federicobond
Copy link
Member Author

Yeah, weird that it worked before. I'm in favor of removing the @abstractmethod decorator from AbstractProvider, as this may have an impact in downstream providers.

@federicobond
Copy link
Member Author

This is now building correctly.

@federicobond
Copy link
Member Author

@gruebel perhaps we can trigger a release after merging this PR?

@federicobond federicobond merged commit 924c489 into open-feature:main Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants