Skip to content

Commit 6f6186e

Browse files
tcarrioMeg McRobertsmatthewelwell
authored
feat: process flag evaluation options in client (#31)
Signed-off-by: Tom Carrio <[email protected]> Co-authored-by: Meg McRoberts <[email protected]> Co-authored-by: Matthew Elwell <[email protected]>
1 parent 4eee603 commit 6f6186e

File tree

9 files changed

+217
-25
lines changed

9 files changed

+217
-25
lines changed

.github/workflows/pullrequest.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ jobs:
1717
runs-on: ubuntu-latest
1818
strategy:
1919
matrix:
20-
container: [ "python:3.8", "python:3.9", "python:3.10" ]
20+
container: [ "python:3.8", "python:3.9", "python:3.10", "python:3.11" ]
2121
container:
2222
image: ${{ matrix.container }}
2323

.gitignore

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,7 @@ coverage.xml
4747
*.pot
4848

4949
# Sphinx documentation
50-
docs/_build/
50+
docs/_build/
51+
52+
# Virtual env directories
53+
.venv

CONTRIBUTING.md

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
# Contributing
2+
3+
## Development
4+
5+
### System Requirements
6+
7+
Python 3.8 and above are required.
8+
9+
### Target version(s)
10+
11+
Python 3.8 and above are supported by the SDK.
12+
13+
### Installation and Dependencies
14+
15+
A [`Makefile`](./Makefile) has been included in the project which should make it straightforward to start the project locally. We utilize virtual environments (see [`virtualenv`](https://docs.python.org/3/tutorial/venv.html)) in order to provide isolated development environments for the project. This reduces the risk of invalid or corrupt global packages. It also integrates nicely with Make, which will detect changes in the `requirements-dev.txt` file and update the virtual environment if any occur.
16+
17+
Run `make init` to initialize the project's virtual environment and install all dev dependencies.
18+
19+
### Testing
20+
21+
Run tests with `make test`.
22+
23+
We use `pytest` for our unit testing, making use of `parametrized` to inject cases at scale.
24+
25+
### Integration tests
26+
27+
These are planned once the SDK has been stabilized and a Flagd provider implemented. At that point, we will utilize the [gherkin integration tests](https://github.com/open-feature/test-harness/blob/main/features/evaluation.feature) to validate against a live, seeded Flagd instance.
28+
29+
### Packaging
30+
31+
We publish to the PyPI repository, where you can find this package at [openfeature-sdk](https://pypi.org/project/openfeature-sdk/).
32+
33+
## Pull Request
34+
35+
All contributions to the OpenFeature project are welcome via GitHub pull requests.
36+
37+
To create a new PR, you will need to first fork the GitHub repository and clone upstream.
38+
39+
```bash
40+
git clone https://github.com/open-feature/python-sdk.git openfeature-python-sdk
41+
```
42+
43+
Navigate to the repository folder
44+
45+
```bash
46+
cd openfeature-python-sdk
47+
```
48+
49+
Add your fork as an origin
50+
51+
```bash
52+
git remote add fork https://github.com/YOUR_GITHUB_USERNAME/python-sdk.git
53+
```
54+
55+
Ensure your development environment is all set up by building and testing
56+
57+
```bash
58+
make
59+
```
60+
61+
To start working on a new feature or bugfix, create a new branch and start working on it.
62+
63+
```bash
64+
git checkout -b feat/NAME_OF_FEATURE
65+
# Make your changes
66+
git commit
67+
git push fork feat/NAME_OF_FEATURE
68+
```
69+
70+
Open a pull request against the main python-sdk repository.
71+
72+
### How to Receive Comments
73+
74+
- If the PR is not ready for review, please mark it as
75+
[`draft`](https://github.blog/2019-02-14-introducing-draft-pull-requests/).
76+
- Make sure all required CI checks are clear.
77+
- Submit small, focused PRs addressing a single concern/issue.
78+
- Make sure the PR title reflects the contribution.
79+
- Write a summary that explains the change.
80+
- Include usage examples in the summary, where applicable.
81+
82+
### How to Get PRs Merged
83+
84+
A PR is considered to be **ready to merge** when:
85+
86+
- Major feedback is resolved.
87+
- Urgent fix can take exception as long as it has been actively communicated.
88+
89+
Any Maintainer can merge the PR once it is **ready to merge**. Note, that some
90+
PRs may not be merged immediately if the repo is in the process of a release and
91+
the maintainers decided to defer the PR to the next release train.
92+
93+
If a PR has been stuck (e.g. there are lots of debates and people couldn't agree
94+
on each other), the owner should try to get people aligned by:
95+
96+
- Consolidating the perspectives and putting a summary in the PR. It is
97+
recommended to add a link into the PR description, which points to a comment
98+
with a summary in the PR conversation.
99+
- Tagging domain experts (by looking at the change history) in the PR asking
100+
for suggestion.
101+
- Reaching out to more people on the [CNCF OpenFeature Slack channel](https://cloud-native.slack.com/archives/C0344AANLA1).
102+
- Stepping back to see if it makes sense to narrow down the scope of the PR or
103+
split it up.
104+
- If none of the above worked and the PR has been stuck for more than 2 weeks,
105+
the owner should bring it to the OpenFeatures [meeting](README.md#contributing).
106+
107+
## Design Choices
108+
109+
As with other OpenFeature SDKs, python-sdk follows the
110+
[openfeature-specification](https://github.com/open-feature/spec).

Makefile

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
VENV = . .venv/bin/activate
2+
3+
.PHONY: all
4+
all: lint test
5+
6+
.PHONY: init
7+
init: .venv
8+
9+
.venv: requirements-dev.txt
10+
test -d .venv || python -m virtualenv .venv
11+
$(VENV); pip install -Ur requirements-dev.txt
12+
13+
.PHONY: test
14+
test: .venv
15+
$(VENV); pytest
16+
17+
.PHONY: lint
18+
lint: .venv
19+
$(VENV); black .
20+
$(VENV); flake8 .
21+
$(VENV); isort .
22+
23+
.PHONY: clean
24+
clean:
25+
@rm -rf .venv
26+
@find -iname "*.pyc" -delete

open_feature/immutable_dict/__init__.py

Whitespace-only changes.
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
class MappingProxyType(dict):
2+
"""
3+
MappingProxyType is an immutable dictionary type, written to
4+
support Python 3.8 with easy transition to 3.12 upon removal
5+
of older versions.
6+
7+
See: https://stackoverflow.com/a/72474524
8+
9+
When upgrading to Python 3.12, you can update all references
10+
from:
11+
`from open_feature.immutable_dict.mapping_proxy_type import MappingProxyType`
12+
13+
to:
14+
`from types import MappingProxyType`
15+
"""
16+
17+
def __hash__(self):
18+
return id(self)
19+
20+
def _immutable(self, *args, **kws):
21+
raise TypeError("immutable instance of dictionary")
22+
23+
__setitem__ = _immutable
24+
__delitem__ = _immutable
25+
clear = _immutable
26+
update = _immutable
27+
setdefault = _immutable
28+
pop = _immutable
29+
popitem = _immutable

open_feature/open_feature_client.py

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
from open_feature.provider.no_op_provider import NoOpProvider
2525
from open_feature.provider.provider import AbstractProvider
2626

27-
2827
GetDetailCallable = typing.Union[
2928
typing.Callable[
3029
[str, bool, typing.Optional[EvaluationContext]], FlagEvaluationDetails[bool]
@@ -236,6 +235,9 @@ def evaluate_flag_details(
236235
if flag_evaluation_options is None:
237236
flag_evaluation_options = FlagEvaluationOptions()
238237

238+
evaluation_hooks = flag_evaluation_options.hooks
239+
hook_hints = flag_evaluation_options.hook_hints
240+
239241
hook_context = HookContext(
240242
flag_key=flag_key,
241243
flag_type=flag_type,
@@ -250,24 +252,19 @@ def evaluate_flag_details(
250252
# in the flag evaluation
251253
# before: API, Client, Invocation, Provider
252254
merged_hooks = (
253-
self.hooks
254-
+ flag_evaluation_options.hooks
255-
+ self.provider.get_provider_hooks()
255+
self.hooks + evaluation_hooks + self.provider.get_provider_hooks()
256256
)
257257
# after, error, finally: Provider, Invocation, Client, API
258-
reversed_merged_hooks = (
259-
self.provider.get_provider_hooks()
260-
+ flag_evaluation_options.hooks
261-
+ self.hooks
262-
)
258+
reversed_merged_hooks = merged_hooks[:]
259+
reversed_merged_hooks.sort()
263260

264261
try:
265262
# https://github.com/open-feature/spec/blob/main/specification/sections/03-evaluation-context.md
266263
# Any resulting evaluation context from a before hook will overwrite
267264
# duplicate fields defined globally, on the client, or in the invocation.
268265
# Requirement 3.2.2, 4.3.4: API.context->client.context->invocation.context
269266
invocation_context = before_hooks(
270-
flag_type, hook_context, merged_hooks, None
267+
flag_type, hook_context, merged_hooks, hook_hints
271268
)
272269
invocation_context = invocation_context.merge(ctx2=evaluation_context)
273270

@@ -284,25 +281,31 @@ def evaluate_flag_details(
284281
)
285282

286283
after_hooks(
287-
flag_type, hook_context, flag_evaluation, reversed_merged_hooks, None
284+
flag_type,
285+
hook_context,
286+
flag_evaluation,
287+
reversed_merged_hooks,
288+
hook_hints,
288289
)
289290

290291
return flag_evaluation
291292

292-
except OpenFeatureError as e:
293-
error_hooks(flag_type, hook_context, e, reversed_merged_hooks, None)
293+
except OpenFeatureError as err:
294+
error_hooks(flag_type, hook_context, err, reversed_merged_hooks, hook_hints)
295+
294296
return FlagEvaluationDetails(
295297
flag_key=flag_key,
296298
value=default_value,
297299
reason=Reason.ERROR,
298-
error_code=e.error_code,
299-
error_message=e.error_message,
300+
error_code=err.error_code,
301+
error_message=err.error_message,
300302
)
301303
# Catch any type of exception here since the user can provide any exception
302304
# in the error hooks
303-
except Exception as e: # noqa
304-
error_hooks(flag_type, hook_context, e, reversed_merged_hooks, None)
305-
error_message = getattr(e, "error_message", str(e))
305+
except Exception as err: # noqa
306+
error_hooks(flag_type, hook_context, err, reversed_merged_hooks, hook_hints)
307+
308+
error_message = getattr(err, "error_message", str(err))
306309
return FlagEvaluationDetails(
307310
flag_key=flag_key,
308311
value=default_value,
@@ -312,7 +315,7 @@ def evaluate_flag_details(
312315
)
313316

314317
finally:
315-
after_all_hooks(flag_type, hook_context, reversed_merged_hooks, None)
318+
after_all_hooks(flag_type, hook_context, reversed_merged_hooks, hook_hints)
316319

317320
def _create_provider_evaluation(
318321
self,

readme.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,3 +83,7 @@ Thanks so much to our contributors.
8383
</a>
8484

8585
Made with [contrib.rocks](https://contrib.rocks).
86+
87+
### Development
88+
89+
If you would like to contribute to the project, please see our [contributing](./CONTRIBUTING.md) documentation!

tests/hooks/test_hook_support.py

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
from unittest.mock import ANY
2+
13
from open_feature.flag_evaluation.flag_evaluation_details import FlagEvaluationDetails
24
from open_feature.flag_evaluation.flag_type import FlagType
35
from open_feature.hooks.hook_context import HookContext
@@ -7,26 +9,33 @@
79
before_hooks,
810
error_hooks,
911
)
12+
from open_feature.immutable_dict.mapping_proxy_type import MappingProxyType
1013

1114

1215
def test_error_hooks_run_error_method(mock_hook):
1316
# Given
1417
hook_context = HookContext("flag_key", FlagType.BOOLEAN, True, "")
18+
hook_hints = MappingProxyType(dict())
1519
# When
16-
error_hooks(FlagType.BOOLEAN, hook_context, Exception, [mock_hook], {})
20+
error_hooks(FlagType.BOOLEAN, hook_context, Exception, [mock_hook], hook_hints)
1721
# Then
1822
mock_hook.supports_flag_value_type.assert_called_once()
1923
mock_hook.error.assert_called_once()
24+
mock_hook.error.assert_called_with(
25+
hook_context=hook_context, exception=ANY, hints=hook_hints
26+
)
2027

2128

2229
def test_before_hooks_run_before_method(mock_hook):
2330
# Given
2431
hook_context = HookContext("flag_key", FlagType.BOOLEAN, True, "")
32+
hook_hints = MappingProxyType(dict())
2533
# When
26-
before_hooks(FlagType.BOOLEAN, hook_context, [mock_hook], {})
34+
before_hooks(FlagType.BOOLEAN, hook_context, [mock_hook], hook_hints)
2735
# Then
2836
mock_hook.supports_flag_value_type.assert_called_once()
2937
mock_hook.before.assert_called_once()
38+
mock_hook.before.assert_called_with(hook_context=hook_context, hints=hook_hints)
3039

3140

3241
def test_after_hooks_run_after_method(mock_hook):
@@ -35,20 +44,28 @@ def test_after_hooks_run_after_method(mock_hook):
3544
flag_evaluation_details = FlagEvaluationDetails(
3645
hook_context.flag_key, "val", "unknown"
3746
)
47+
hook_hints = MappingProxyType(dict())
3848
# When
3949
after_hooks(
40-
FlagType.BOOLEAN, hook_context, flag_evaluation_details, [mock_hook], {}
50+
FlagType.BOOLEAN, hook_context, flag_evaluation_details, [mock_hook], hook_hints
4151
)
4252
# Then
4353
mock_hook.supports_flag_value_type.assert_called_once()
4454
mock_hook.after.assert_called_once()
55+
mock_hook.after.assert_called_with(
56+
hook_context=hook_context, details=flag_evaluation_details, hints=hook_hints
57+
)
4558

4659

4760
def test_finally_after_hooks_run_finally_after_method(mock_hook):
4861
# Given
4962
hook_context = HookContext("flag_key", FlagType.BOOLEAN, True, "")
63+
hook_hints = MappingProxyType(dict())
5064
# When
51-
after_all_hooks(FlagType.BOOLEAN, hook_context, [mock_hook], {})
65+
after_all_hooks(FlagType.BOOLEAN, hook_context, [mock_hook], hook_hints)
5266
# Then
5367
mock_hook.supports_flag_value_type.assert_called_once()
5468
mock_hook.finally_after.assert_called_once()
69+
mock_hook.finally_after.assert_called_with(
70+
hook_context=hook_context, hints=hook_hints
71+
)

0 commit comments

Comments
 (0)