Skip to content
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: process flag evaluation options in client #31

Merged

Conversation

tcarrio
Copy link
Member

@tcarrio tcarrio commented Oct 29, 2022

This PR

  • performs validation of options as dictionary before pulling hooks and hints
  • introduces immutable dictionary extension of dict following naming from 3.12 release
  • documented "upgrade" process for MappingProxyType

Related Issues

Provides additional steps towards spec compliance.

Notes

Follow-up Tasks

Hooks are completely unused still, but I'm chunking out separate PRs of work.

How to test

Added unit tests where applicable (nothing exists yet for the client though)

@tcarrio tcarrio force-pushed the feat/process-flag-evaluation-options branch from 21cdc21 to 2325248 Compare October 29, 2022 06:02
@tcarrio
Copy link
Member Author

tcarrio commented Oct 29, 2022

Signed off on commits

@tcarrio tcarrio mentioned this pull request Oct 29, 2022
@tcarrio tcarrio force-pushed the feat/process-flag-evaluation-options branch from 27f0e32 to 2efb839 Compare November 8, 2022 02:44
- performs validation of options as dictionary before pulling hooks and hints
- introduces immutable dictionary extension of dict following naming from 3.12 release
- documented "upgrade" process for MappingProxyType

Signed-off-by: Tom Carrio <[email protected]>
- commonly stored in .venv as the initial dir to support

Signed-off-by: Tom Carrio <[email protected]>
@tcarrio tcarrio force-pushed the feat/process-flag-evaluation-options branch from 2efb839 to 6d86aa1 Compare November 8, 2022 02:46
Signed-off-by: Tom Carrio <[email protected]>
@tcarrio
Copy link
Member Author

tcarrio commented Nov 21, 2022

This needs a small fix in the typing to resolve the build issues, but otherwise this is a compatible change that makes the hooks inputs actually immutable, so worthwhile to get this working again.

@tcarrio tcarrio force-pushed the feat/process-flag-evaluation-options branch from 0ed59b7 to 297c0c2 Compare November 22, 2022 22:44
@codecov-commenter
Copy link

codecov-commenter commented Nov 22, 2022

Codecov Report

Merging #31 (d7d3ac3) into main (3dc3a9d) will decrease coverage by 0.27%.
The diff coverage is 90.90%.

@@            Coverage Diff             @@
##             main      #31      +/-   ##
==========================================
- Coverage   92.92%   92.64%   -0.28%     
==========================================
  Files          18       19       +1     
  Lines         325      340      +15     
==========================================
+ Hits          302      315      +13     
- Misses         23       25       +2     
Flag Coverage Δ
unittests 92.64% <90.90%> (-0.28%) ⬇️

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

Impacted Files Coverage Δ
open_feature/immutable_dict/mapping_proxy_type.py 83.33% <83.33%> (ø)
open_feature/open_feature_client.py 96.42% <100.00%> (+0.13%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

CONTRIBUTING.md Show resolved Hide resolved
open_feature/open_feature_client.py Outdated Show resolved Hide resolved
open_feature/open_feature_client.py Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
Co-authored-by: Meg McRoberts <[email protected]>
Signed-off-by: Tom Carrio <[email protected]>
CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
open_feature/open_feature_client.py Outdated Show resolved Hide resolved
open_feature/open_feature_client.py Outdated Show resolved Hide resolved
tcarrio and others added 4 commits December 1, 2022 23:59
@tcarrio tcarrio force-pushed the feat/process-flag-evaluation-options branch from 0d250c7 to 01d3214 Compare December 2, 2022 05:00
Signed-off-by: Tom Carrio <[email protected]>
@tcarrio
Copy link
Member Author

tcarrio commented Dec 2, 2022

The last outstanding work for this review seems to be utilizing a different build tool besides Make. I created a separate issue to track that work.

In the meantime would we prefer to remove Make until Poetry/pipenv/etc. is implemented or leave it to be replaced when that issue is taken on?

@beeme1mr beeme1mr merged commit 6f6186e into open-feature:main Dec 2, 2022
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.

7 participants