From ca0b6f48a8f3399fdceaf1c8d2aa90be654b4384 Mon Sep 17 00:00:00 2001 From: Tom Carrio Date: Sat, 29 Oct 2022 01:44:09 -0400 Subject: [PATCH 01/16] feat: process flag evaluation options in client - 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 --- open_feature/immutable_dict/__init__.py | 0 .../immutable_dict/mapping_proxy_type.py | 28 +++++++++++++++++++ open_feature/open_feature_client.py | 26 +++++++++++++---- 3 files changed, 49 insertions(+), 5 deletions(-) create mode 100644 open_feature/immutable_dict/__init__.py create mode 100644 open_feature/immutable_dict/mapping_proxy_type.py diff --git a/open_feature/immutable_dict/__init__.py b/open_feature/immutable_dict/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/open_feature/immutable_dict/mapping_proxy_type.py b/open_feature/immutable_dict/mapping_proxy_type.py new file mode 100644 index 00000000..389b4688 --- /dev/null +++ b/open_feature/immutable_dict/mapping_proxy_type.py @@ -0,0 +1,28 @@ +class MappingProxyType(dict): + """ + MappingProxyType is an immutable dictionary type, written to + support Python 3.8 with easy transition to 3.12 upon removal + of older versions. + + See: https://stackoverflow.com/a/72474524 + + When upgrading to Python 3.12, you can update all references + from: + `from open_feature.immutable_dict.mapping_proxy_type import MappingProxyType` + + to: + `from types import MappingProxyType` + """ + def __hash__(self): + return id(self) + + def _immutable(self, *args, **kws): + raise TypeError('immutable instance of dictionary') + + __setitem__ = _immutable + __delitem__ = _immutable + clear = _immutable + update = _immutable + setdefault = _immutable + pop = _immutable + popitem = _immutable diff --git a/open_feature/open_feature_client.py b/open_feature/open_feature_client.py index 8be455ef..15ca661d 100644 --- a/open_feature/open_feature_client.py +++ b/open_feature/open_feature_client.py @@ -16,6 +16,7 @@ before_hooks, error_hooks, ) +from open_feature.immutable_dict.mapping_proxy_type import MappingProxyType from open_feature.open_feature_evaluation_context import api_evaluation_context from open_feature.provider.no_op_provider import NoOpProvider from open_feature.provider.provider import AbstractProvider @@ -182,6 +183,8 @@ def evaluate_flag_details( if evaluation_context is None: evaluation_context = EvaluationContext() + evaluation_hooks, hook_hints = self.__extract_evaluation_options(flag_evaluation_options) + hook_context = HookContext( flag_key=flag_key, flag_type=flag_type, @@ -197,7 +200,7 @@ def evaluate_flag_details( # Any resulting evaluation context from a before hook will overwrite # duplicate fields defined globally, on the client, or in the invocation. invocation_context = before_hooks( - flag_type, hook_context, merged_hooks, None + flag_type, hook_context, merged_hooks, hook_hints ) invocation_context.merge(ctx2=evaluation_context) @@ -213,12 +216,12 @@ def evaluate_flag_details( merged_context, ) - after_hooks(type, hook_context, flag_evaluation, merged_hooks, None) + after_hooks(type, hook_context, flag_evaluation, merged_hooks, hook_hints) return flag_evaluation except OpenFeatureError as e: - error_hooks(flag_type, hook_context, e, merged_hooks, None) + error_hooks(flag_type, hook_context, e, merged_hooks, hook_hints) return FlagEvaluationDetails( flag_key=flag_key, value=default_value, @@ -229,7 +232,7 @@ def evaluate_flag_details( # Catch any type of exception here since the user can provide any exception # in the error hooks except Exception as e: # noqa - error_hooks(flag_type, hook_context, e, merged_hooks, None) + error_hooks(flag_type, hook_context, e, merged_hooks, hook_hints) error_message = getattr(e, "error_message", str(e)) return FlagEvaluationDetails( flag_key=flag_key, @@ -240,7 +243,7 @@ def evaluate_flag_details( ) finally: - after_all_hooks(flag_type, hook_context, merged_hooks, None) + after_all_hooks(flag_type, hook_context, merged_hooks, hook_hints) def _create_provider_evaluation( self, @@ -280,3 +283,16 @@ def _create_provider_evaluation( raise GeneralError(error_message="Unknown flag type") return get_details_callable(*args) + + def __extract_evaluation_options(self, flag_evaluation_options: typing.Any) -> typing.Tuple(typing.List[Hook], MappingProxyType): + evaluation_hooks: typing.List[Hook] = [] + hook_hints: dict = {} + + if flag_evaluation_options is dict: + if 'hook_hints' in flag_evaluation_options and flag_evaluation_options['hook_hints'] is dict: + hook_hints = dict(flag_evaluation_options['hook_hints']) + + if 'hooks' in flag_evaluation_options and flag_evaluation_options['hooks'] is list: + evaluation_hooks = flag_evaluation_options['hooks'] + + return (evaluation_hooks, MappingProxyType(hook_hints)) \ No newline at end of file From 63825b16cf80daabfc76dbccd1e6d6b762adbb1f Mon Sep 17 00:00:00 2001 From: Tom Carrio Date: Sat, 29 Oct 2022 01:58:29 -0400 Subject: [PATCH 02/16] test: add unit tests to assert hook_hints are called in hooks Signed-off-by: Tom Carrio --- tests/hooks/test_hook_support.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/tests/hooks/test_hook_support.py b/tests/hooks/test_hook_support.py index fe906cd6..e398445c 100644 --- a/tests/hooks/test_hook_support.py +++ b/tests/hooks/test_hook_support.py @@ -7,26 +7,31 @@ before_hooks, error_hooks, ) +from unittest.mock import ANY def test_error_hooks_run_error_method(mock_hook): # Given hook_context = HookContext("flag_key", FlagType.BOOLEAN, True, "") + hook_hints = MappingProxyType(dict()) # When - error_hooks(FlagType.BOOLEAN, hook_context, Exception, [mock_hook], {}) + error_hooks(FlagType.BOOLEAN, hook_context, Exception, [mock_hook], hook_hints) # Then mock_hook.supports_flag_value_type.assert_called_once() mock_hook.error.assert_called_once() + mock_hook.error.assert_called_with(hook_context, ANY, hook_hints) def test_before_hooks_run_before_method(mock_hook): # Given hook_context = HookContext("flag_key", FlagType.BOOLEAN, True, "") + hook_hints = MappingProxyType(dict()) # When - before_hooks(FlagType.BOOLEAN, hook_context, [mock_hook], {}) + before_hooks(FlagType.BOOLEAN, hook_context, [mock_hook], hook_hints) # Then mock_hook.supports_flag_value_type.assert_called_once() mock_hook.before.assert_called_once() + mock_hook.error.assert_called_with(hook_context, hook_hints) def test_after_hooks_run_after_method(mock_hook): @@ -35,20 +40,24 @@ def test_after_hooks_run_after_method(mock_hook): flag_evaluation_details = FlagEvaluationDetails( hook_context.flag_key, "val", "unknown" ) + hook_hints = MappingProxyType(dict()) # When after_hooks( - FlagType.BOOLEAN, hook_context, flag_evaluation_details, [mock_hook], {} + FlagType.BOOLEAN, hook_context, flag_evaluation_details, [mock_hook], hook_hints ) # Then mock_hook.supports_flag_value_type.assert_called_once() mock_hook.after.assert_called_once() + mock_hook.error.assert_called_with(hook_context, flag_evaluation_details, hook_hints) def test_finally_after_hooks_run_finally_after_method(mock_hook): # Given hook_context = HookContext("flag_key", FlagType.BOOLEAN, True, "") + hook_hints = MappingProxyType(dict()) # When - after_all_hooks(FlagType.BOOLEAN, hook_context, [mock_hook], {}) + after_all_hooks(FlagType.BOOLEAN, hook_context, [mock_hook], hook_hints) # Then mock_hook.supports_flag_value_type.assert_called_once() mock_hook.finally_after.assert_called_once() + mock_hook.error.assert_called_with(hook_context, hook_hints) From 7ffc959fc6469ce2370256062471706e582c3f6a Mon Sep 17 00:00:00 2001 From: Tom Carrio Date: Mon, 7 Nov 2022 21:44:11 -0500 Subject: [PATCH 03/16] build: ignore venv directories - commonly stored in .venv as the initial dir to support Signed-off-by: Tom Carrio --- .gitignore | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 5abdfe09..6e19874f 100644 --- a/.gitignore +++ b/.gitignore @@ -47,4 +47,7 @@ coverage.xml *.pot # Sphinx documentation -docs/_build/ \ No newline at end of file +docs/_build/ + +# Virtual env directories +.venv \ No newline at end of file From 6d86aa1c24d3aacfbb37b39bfa0092bcdae24e12 Mon Sep 17 00:00:00 2001 From: Tom Carrio Date: Mon, 7 Nov 2022 21:44:20 -0500 Subject: [PATCH 04/16] style: applied black formatting Signed-off-by: Tom Carrio --- .../immutable_dict/mapping_proxy_type.py | 13 +++++---- open_feature/open_feature_client.py | 28 +++++++++++++------ tests/hooks/test_hook_support.py | 4 ++- 3 files changed, 29 insertions(+), 16 deletions(-) diff --git a/open_feature/immutable_dict/mapping_proxy_type.py b/open_feature/immutable_dict/mapping_proxy_type.py index 389b4688..6bee4924 100644 --- a/open_feature/immutable_dict/mapping_proxy_type.py +++ b/open_feature/immutable_dict/mapping_proxy_type.py @@ -13,16 +13,17 @@ class MappingProxyType(dict): to: `from types import MappingProxyType` """ + def __hash__(self): return id(self) def _immutable(self, *args, **kws): - raise TypeError('immutable instance of dictionary') + raise TypeError("immutable instance of dictionary") __setitem__ = _immutable __delitem__ = _immutable - clear = _immutable - update = _immutable - setdefault = _immutable - pop = _immutable - popitem = _immutable + clear = _immutable + update = _immutable + setdefault = _immutable + pop = _immutable + popitem = _immutable diff --git a/open_feature/open_feature_client.py b/open_feature/open_feature_client.py index 15ca661d..02ac08a5 100644 --- a/open_feature/open_feature_client.py +++ b/open_feature/open_feature_client.py @@ -183,7 +183,9 @@ def evaluate_flag_details( if evaluation_context is None: evaluation_context = EvaluationContext() - evaluation_hooks, hook_hints = self.__extract_evaluation_options(flag_evaluation_options) + evaluation_hooks, hook_hints = self.__extract_evaluation_options( + flag_evaluation_options + ) hook_context = HookContext( flag_key=flag_key, @@ -284,15 +286,23 @@ def _create_provider_evaluation( return get_details_callable(*args) - def __extract_evaluation_options(self, flag_evaluation_options: typing.Any) -> typing.Tuple(typing.List[Hook], MappingProxyType): + def __extract_evaluation_options( + self, flag_evaluation_options: typing.Any + ) -> typing.Tuple(typing.List[Hook], MappingProxyType): evaluation_hooks: typing.List[Hook] = [] hook_hints: dict = {} if flag_evaluation_options is dict: - if 'hook_hints' in flag_evaluation_options and flag_evaluation_options['hook_hints'] is dict: - hook_hints = dict(flag_evaluation_options['hook_hints']) - - if 'hooks' in flag_evaluation_options and flag_evaluation_options['hooks'] is list: - evaluation_hooks = flag_evaluation_options['hooks'] - - return (evaluation_hooks, MappingProxyType(hook_hints)) \ No newline at end of file + if ( + "hook_hints" in flag_evaluation_options + and flag_evaluation_options["hook_hints"] is dict + ): + hook_hints = dict(flag_evaluation_options["hook_hints"]) + + if ( + "hooks" in flag_evaluation_options + and flag_evaluation_options["hooks"] is list + ): + evaluation_hooks = flag_evaluation_options["hooks"] + + return (evaluation_hooks, MappingProxyType(hook_hints)) diff --git a/tests/hooks/test_hook_support.py b/tests/hooks/test_hook_support.py index e398445c..a0b492fd 100644 --- a/tests/hooks/test_hook_support.py +++ b/tests/hooks/test_hook_support.py @@ -48,7 +48,9 @@ def test_after_hooks_run_after_method(mock_hook): # Then mock_hook.supports_flag_value_type.assert_called_once() mock_hook.after.assert_called_once() - mock_hook.error.assert_called_with(hook_context, flag_evaluation_details, hook_hints) + mock_hook.error.assert_called_with( + hook_context, flag_evaluation_details, hook_hints + ) def test_finally_after_hooks_run_finally_after_method(mock_hook): From c1addb49e1aade18966be81c385910d1e79f6157 Mon Sep 17 00:00:00 2001 From: Tom Carrio Date: Mon, 7 Nov 2022 22:34:04 -0500 Subject: [PATCH 05/16] style: run isort formatter Signed-off-by: Tom Carrio --- tests/hooks/test_hook_support.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/hooks/test_hook_support.py b/tests/hooks/test_hook_support.py index a0b492fd..5a5d8e3f 100644 --- a/tests/hooks/test_hook_support.py +++ b/tests/hooks/test_hook_support.py @@ -1,3 +1,5 @@ +from unittest.mock import ANY + from open_feature.flag_evaluation.flag_evaluation_details import FlagEvaluationDetails from open_feature.flag_evaluation.flag_type import FlagType from open_feature.hooks.hook_context import HookContext @@ -7,7 +9,7 @@ before_hooks, error_hooks, ) -from unittest.mock import ANY +from open_feature.immutable_dict import MappingProxyType def test_error_hooks_run_error_method(mock_hook): From 4e478c42e7d1a35386cb412db6dcfab2c8d0113e Mon Sep 17 00:00:00 2001 From: Tom Carrio Date: Mon, 21 Nov 2022 17:44:18 -0500 Subject: [PATCH 06/16] fix: typing reference using square brackets Signed-off-by: Tom Carrio --- open_feature/open_feature_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/open_feature/open_feature_client.py b/open_feature/open_feature_client.py index a6dbdb3d..c8f89beb 100644 --- a/open_feature/open_feature_client.py +++ b/open_feature/open_feature_client.py @@ -365,7 +365,7 @@ def _create_provider_evaluation( def __extract_evaluation_options( self, flag_evaluation_options: typing.Any - ) -> typing.Tuple(typing.List[Hook], MappingProxyType): + ) -> typing.Tuple[typing.List[Hook], MappingProxyType]: evaluation_hooks: typing.List[Hook] = [] hook_hints: dict = {} From 63619164c1e78839d205db2939ee2586412ffb24 Mon Sep 17 00:00:00 2001 From: Tom Carrio Date: Mon, 21 Nov 2022 17:47:47 -0500 Subject: [PATCH 07/16] fix: include closing parens on after_hooks call Signed-off-by: Tom Carrio --- open_feature/open_feature_client.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/open_feature/open_feature_client.py b/open_feature/open_feature_client.py index c8f89beb..e9ab0686 100644 --- a/open_feature/open_feature_client.py +++ b/open_feature/open_feature_client.py @@ -286,7 +286,8 @@ def evaluate_flag_details( ) after_hooks( - flag_type, hook_context, flag_evaluation, reversed_merged_hooks, hook_hints + flag_type, hook_context, flag_evaluation, reversed_merged_hooks, hook_hints, + ) return flag_evaluation From a18481c75ed00878eb2c88891da820566cab30f5 Mon Sep 17 00:00:00 2001 From: Tom Carrio Date: Tue, 22 Nov 2022 17:27:26 -0500 Subject: [PATCH 08/16] fix: running test suite with immutable hook hints Signed-off-by: Tom Carrio --- open_feature/open_feature_client.py | 17 ++++++++--------- tests/hooks/test_hook_support.py | 16 ++++++++++------ 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/open_feature/open_feature_client.py b/open_feature/open_feature_client.py index e9ab0686..d6e2cf7b 100644 --- a/open_feature/open_feature_client.py +++ b/open_feature/open_feature_client.py @@ -25,7 +25,6 @@ from open_feature.provider.no_op_provider import NoOpProvider from open_feature.provider.provider import AbstractProvider - GetDetailCallable = typing.Union[ typing.Callable[ [str, bool, typing.Optional[EvaluationContext]], FlagEvaluationDetails[bool] @@ -252,15 +251,11 @@ def evaluate_flag_details( # in the flag evaluation # before: API, Client, Invocation, Provider merged_hooks = ( - self.hooks - + flag_evaluation_options.hooks - + self.provider.get_provider_hooks() + self.hooks + evaluation_hooks + self.provider.get_provider_hooks() ) # after, error, finally: Provider, Invocation, Client, API reversed_merged_hooks = ( - self.provider.get_provider_hooks() - + flag_evaluation_options.hooks - + self.hooks + self.provider.get_provider_hooks() + evaluation_hooks + self.hooks ) try: @@ -286,7 +281,11 @@ def evaluate_flag_details( ) after_hooks( - flag_type, hook_context, flag_evaluation, reversed_merged_hooks, hook_hints, + flag_type, + hook_context, + flag_evaluation, + reversed_merged_hooks, + hook_hints, ) return flag_evaluation @@ -363,7 +362,7 @@ def _create_provider_evaluation( raise TypeMismatchError() return value - + def __extract_evaluation_options( self, flag_evaluation_options: typing.Any ) -> typing.Tuple[typing.List[Hook], MappingProxyType]: diff --git a/tests/hooks/test_hook_support.py b/tests/hooks/test_hook_support.py index 5a5d8e3f..28914c4b 100644 --- a/tests/hooks/test_hook_support.py +++ b/tests/hooks/test_hook_support.py @@ -9,7 +9,7 @@ before_hooks, error_hooks, ) -from open_feature.immutable_dict import MappingProxyType +from open_feature.immutable_dict.mapping_proxy_type import MappingProxyType def test_error_hooks_run_error_method(mock_hook): @@ -21,7 +21,9 @@ def test_error_hooks_run_error_method(mock_hook): # Then mock_hook.supports_flag_value_type.assert_called_once() mock_hook.error.assert_called_once() - mock_hook.error.assert_called_with(hook_context, ANY, hook_hints) + mock_hook.error.assert_called_with( + hook_context=hook_context, exception=ANY, hints=hook_hints + ) def test_before_hooks_run_before_method(mock_hook): @@ -33,7 +35,7 @@ def test_before_hooks_run_before_method(mock_hook): # Then mock_hook.supports_flag_value_type.assert_called_once() mock_hook.before.assert_called_once() - mock_hook.error.assert_called_with(hook_context, hook_hints) + mock_hook.before.assert_called_with(hook_context=hook_context, hints=hook_hints) def test_after_hooks_run_after_method(mock_hook): @@ -50,8 +52,8 @@ def test_after_hooks_run_after_method(mock_hook): # Then mock_hook.supports_flag_value_type.assert_called_once() mock_hook.after.assert_called_once() - mock_hook.error.assert_called_with( - hook_context, flag_evaluation_details, hook_hints + mock_hook.after.assert_called_with( + hook_context=hook_context, details=flag_evaluation_details, hints=hook_hints ) @@ -64,4 +66,6 @@ def test_finally_after_hooks_run_finally_after_method(mock_hook): # Then mock_hook.supports_flag_value_type.assert_called_once() mock_hook.finally_after.assert_called_once() - mock_hook.error.assert_called_with(hook_context, hook_hints) + mock_hook.finally_after.assert_called_with( + hook_context=hook_context, hints=hook_hints + ) From 34d7892d54f6929b38cef570ff9af00e81c27890 Mon Sep 17 00:00:00 2001 From: Tom Carrio Date: Tue, 22 Nov 2022 17:27:52 -0500 Subject: [PATCH 09/16] build: add Makefile for build tasks Signed-off-by: Tom Carrio --- Makefile | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 Makefile diff --git a/Makefile b/Makefile new file mode 100644 index 00000000..81d8eef2 --- /dev/null +++ b/Makefile @@ -0,0 +1,23 @@ +VENV = . .venv/bin/activate + +.venv: requirements-dev.txt + test -d .venv || python -m virtualenv .venv + $(VENV); pip install -Ur requirements-dev.txt + +.PHONY: test +test: .venv + $(VENV); pytest + +.PHONY: lint +lint: .venv + $(VENV); black . + $(VENV); flake8 . + $(VENV); isort . + +.PHONY: clean +clean: + @rm -rf .venv + @find -iname "*.pyc" -delete + +.PHONY: all +all: lint test From 297c0c2177552afaa4d1c4fea5468611ab069c15 Mon Sep 17 00:00:00 2001 From: Tom Carrio Date: Tue, 22 Nov 2022 17:41:06 -0500 Subject: [PATCH 10/16] docs: contributing information in README, new file, touch-ups for Makefile Signed-off-by: Tom Carrio --- CONTRIBUTING.md | 110 ++++++++++++++++++++++++++++++++++++++++++++++++ Makefile | 9 ++-- readme.md | 4 ++ 3 files changed, 120 insertions(+), 3 deletions(-) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 00000000..855037f2 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,110 @@ +# Contributing + +## Development + +### System Requirements + +Python 3.8 and above are required. + +### Target version(s) + +Python 3.8 and above are supported by the SDK. + +### Installation and Dependencies + +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. + +Run `make init` to initialize the project's virtual environment and install all dev dependencies. + +### Testing + +Run tests with `make test`. + +We use `pytest` for our unit testing, making use of `parametrized` to inject cases at scale. + +### Integration tests + +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. + +### Packaging + +We publish to the PyPI repository, where you can find this package at [openfeature-sdk](https://pypi.org/project/openfeature-sdk/). + +## Pull Request + +All contributions to the OpenFeature project are welcome via GitHub pull requests. + +To create a new PR, you will need to first fork the GitHub repository and clone upstream. + +```bash +git clone https://github.com/open-feature/python-sdk.git openfeature-python-sdk +``` + +Navigate to the repository folder + +```bash +cd openfeature-python-sdk +``` + +Add your fork as an origin + +```bash +git remote add fork https://github.com/YOUR_GITHUB_USERNAME/python-sdk.git +``` + +Makes sure your development environment is all setup by building and testing + +```bash +make +``` + +To start working on a new feature or bugfix, create a new branch and start working on it. + +```bash +git checkout -b feat/NAME_OF_FEATURE +# Make your changes +git commit +git push fork feat/NAME_OF_FEATURE +``` + +Open a pull request against the main js-sdk repository. + +### How to Receive Comments + +- If the PR is not ready for review, please mark it as + [`draft`](https://github.blog/2019-02-14-introducing-draft-pull-requests/). +- Make sure all required CI checks are clear. +- Submit small, focused PRs addressing a single concern/issue. +- Make sure the PR title reflects the contribution. +- Write a summary that helps understand the change. +- Include usage examples in the summary, where applicable. + +### How to Get PRs Merged + +A PR is considered to be **ready to merge** when: + +- Major feedback is resolved. +- Urgent fix can take exception as long as it has been actively communicated. + +Any Maintainer can merge the PR once it is **ready to merge**. Note, that some +PRs may not be merged immediately if the repo is in the process of a release and +the maintainers decided to defer the PR to the next release train. + +If a PR has been stuck (e.g. there are lots of debates and people couldn't agree +on each other), the owner should try to get people aligned by: + +- Consolidating the perspectives and putting a summary in the PR. It is + recommended to add a link into the PR description, which points to a comment + with a summary in the PR conversation. +- Tagging domain experts (by looking at the change history) in the PR asking + for suggestion. +- Reaching out to more people on the [CNCF OpenFeature Slack channel](https://cloud-native.slack.com/archives/C0344AANLA1). +- Stepping back to see if it makes sense to narrow down the scope of the PR or + split it up. +- If none of the above worked and the PR has been stuck for more than 2 weeks, + the owner should bring it to the OpenFeatures [meeting](README.md#contributing). + +## Design Choices + +As with other OpenFeature SDKs, python-sdk follows the +[openfeature-specification](https://github.com/open-feature/spec). \ No newline at end of file diff --git a/Makefile b/Makefile index 81d8eef2..12d7185b 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,11 @@ VENV = . .venv/bin/activate +.PHONY: all +all: lint test + +.PHONY: init +init: .venv + .venv: requirements-dev.txt test -d .venv || python -m virtualenv .venv $(VENV); pip install -Ur requirements-dev.txt @@ -18,6 +24,3 @@ lint: .venv clean: @rm -rf .venv @find -iname "*.pyc" -delete - -.PHONY: all -all: lint test diff --git a/readme.md b/readme.md index 3e457aa2..cbeb9e3e 100644 --- a/readme.md +++ b/readme.md @@ -83,3 +83,7 @@ Thanks so much to our contributors. Made with [contrib.rocks](https://contrib.rocks). + +### Development + +If you would like to contribute to the project, please see our [contributing](./CONTRIBUTING.md) documentation! From c4489dc7e0f66971dbf228b310940bab47eaa7ea Mon Sep 17 00:00:00 2001 From: Tom Carrio Date: Wed, 23 Nov 2022 17:01:27 -0500 Subject: [PATCH 11/16] docs: update CONTRIBUTING.md Co-authored-by: Meg McRoberts Signed-off-by: Tom Carrio --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 855037f2..ec6fb289 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -52,7 +52,7 @@ Add your fork as an origin git remote add fork https://github.com/YOUR_GITHUB_USERNAME/python-sdk.git ``` -Makes sure your development environment is all setup by building and testing +Ensure your development environment is all set up by building and testing ```bash make From 4fedcc8d9763bbe3bee2b1c1e8cd9461a527817f Mon Sep 17 00:00:00 2001 From: Tom Carrio Date: Wed, 23 Nov 2022 17:01:41 -0500 Subject: [PATCH 12/16] docs: update CONTRIBUTING.md Co-authored-by: Meg McRoberts Signed-off-by: Tom Carrio --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ec6fb289..1b729d6a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -76,7 +76,7 @@ Open a pull request against the main js-sdk repository. - Make sure all required CI checks are clear. - Submit small, focused PRs addressing a single concern/issue. - Make sure the PR title reflects the contribution. -- Write a summary that helps understand the change. +- Write a summary that explains the change. - Include usage examples in the summary, where applicable. ### How to Get PRs Merged From 2f5c73c29074d3df880e79c785f45643e03ec201 Mon Sep 17 00:00:00 2001 From: Tom Carrio Date: Tue, 22 Nov 2022 17:44:35 -0500 Subject: [PATCH 13/16] ci: support for Python 3.11 in PR action Signed-off-by: Tom Carrio --- .github/workflows/pullrequest.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pullrequest.yml b/.github/workflows/pullrequest.yml index 5be9b650..aca55ae6 100644 --- a/.github/workflows/pullrequest.yml +++ b/.github/workflows/pullrequest.yml @@ -17,7 +17,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - container: [ "python:3.8", "python:3.9", "python:3.10" ] + container: [ "python:3.8", "python:3.9", "python:3.10", "python:3.11" ] container: image: ${{ matrix.container }} From eb5ce6bfae4864db9501135ade7148d50f3b794f Mon Sep 17 00:00:00 2001 From: Tom Carrio Date: Thu, 1 Dec 2022 23:54:03 -0500 Subject: [PATCH 14/16] feat: reverse the original hooks list and utilize FlagEvaluationContext Signed-off-by: Tom Carrio --- open_feature/open_feature_client.py | 47 +++++++++-------------------- 1 file changed, 14 insertions(+), 33 deletions(-) diff --git a/open_feature/open_feature_client.py b/open_feature/open_feature_client.py index d6e2cf7b..f5f7e181 100644 --- a/open_feature/open_feature_client.py +++ b/open_feature/open_feature_client.py @@ -233,9 +233,11 @@ def evaluate_flag_details( if evaluation_context is None: evaluation_context = EvaluationContext() - evaluation_hooks, hook_hints = self.__extract_evaluation_options( - flag_evaluation_options - ) + if flag_evaluation_options is None: + flag_evaluation_options = FlagEvaluationOptions() + + evaluation_hooks = flag_evaluation_options.hooks + hook_hints = flag_evaluation_options.hook_hints hook_context = HookContext( flag_key=flag_key, @@ -254,9 +256,8 @@ def evaluate_flag_details( self.hooks + evaluation_hooks + self.provider.get_provider_hooks() ) # after, error, finally: Provider, Invocation, Client, API - reversed_merged_hooks = ( - self.provider.get_provider_hooks() + evaluation_hooks + self.hooks - ) + reversed_merged_hooks = merged_hooks[:] + reversed_merged_hooks.sort() try: # https://github.com/open-feature/spec/blob/main/specification/sections/03-evaluation-context.md @@ -290,22 +291,22 @@ def evaluate_flag_details( return flag_evaluation - except OpenFeatureError as e: - error_hooks(flag_type, hook_context, e, reversed_merged_hooks, hook_hints) + except OpenFeatureError as err: + error_hooks(flag_type, hook_context, err, reversed_merged_hooks, hook_hints) return FlagEvaluationDetails( flag_key=flag_key, value=default_value, reason=Reason.ERROR, - error_code=e.error_code, - error_message=e.error_message, + error_code=err.error_code, + error_message=err.error_message, ) # Catch any type of exception here since the user can provide any exception # in the error hooks - except Exception as e: # noqa - error_hooks(flag_type, hook_context, e, reversed_merged_hooks, hook_hints) + except Exception as err: # noqa + error_hooks(flag_type, hook_context, err, reversed_merged_hooks, hook_hints) - error_message = getattr(e, "error_message", str(e)) + error_message = getattr(err, "error_message", str(err)) return FlagEvaluationDetails( flag_key=flag_key, value=default_value, @@ -363,23 +364,3 @@ def _create_provider_evaluation( return value - def __extract_evaluation_options( - self, flag_evaluation_options: typing.Any - ) -> typing.Tuple[typing.List[Hook], MappingProxyType]: - evaluation_hooks: typing.List[Hook] = [] - hook_hints: dict = {} - - if flag_evaluation_options is dict: - if ( - "hook_hints" in flag_evaluation_options - and flag_evaluation_options["hook_hints"] is dict - ): - hook_hints = dict(flag_evaluation_options["hook_hints"]) - - if ( - "hooks" in flag_evaluation_options - and flag_evaluation_options["hooks"] is list - ): - evaluation_hooks = flag_evaluation_options["hooks"] - - return (evaluation_hooks, MappingProxyType(hook_hints)) From 01d3214d40a00e9daa99510a1bd74423b1f74b9c Mon Sep 17 00:00:00 2001 From: Tom Carrio Date: Thu, 1 Dec 2022 23:57:21 -0500 Subject: [PATCH 15/16] docs: reference python-sdk Co-authored-by: Matthew Elwell Signed-off-by: Tom Carrio --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1b729d6a..fdd2d94b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -67,7 +67,7 @@ git commit git push fork feat/NAME_OF_FEATURE ``` -Open a pull request against the main js-sdk repository. +Open a pull request against the main python-sdk repository. ### How to Receive Comments From d7d3ac3578c5c6abc54d8ab3af23ecfc6a8294b5 Mon Sep 17 00:00:00 2001 From: Tom Carrio Date: Fri, 2 Dec 2022 00:02:17 -0500 Subject: [PATCH 16/16] fix: remove unused import Signed-off-by: Tom Carrio --- open_feature/open_feature_client.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/open_feature/open_feature_client.py b/open_feature/open_feature_client.py index f5f7e181..1090db0c 100644 --- a/open_feature/open_feature_client.py +++ b/open_feature/open_feature_client.py @@ -20,7 +20,6 @@ before_hooks, error_hooks, ) -from open_feature.immutable_dict.mapping_proxy_type import MappingProxyType from open_feature.open_feature_evaluation_context import api_evaluation_context from open_feature.provider.no_op_provider import NoOpProvider from open_feature.provider.provider import AbstractProvider @@ -363,4 +362,3 @@ def _create_provider_evaluation( raise TypeMismatchError() return value -