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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/pullrequest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}

Expand Down
5 changes: 4 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,7 @@ coverage.xml
*.pot

# Sphinx documentation
docs/_build/
docs/_build/

# Virtual env directories
.venv
110 changes: 110 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -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.
beeme1mr marked this conversation as resolved.
Show resolved Hide resolved

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
```

Ensure your development environment is all set up 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
matthewelwell marked this conversation as resolved.
Show resolved Hide resolved
# Make your changes
git commit
git push fork feat/NAME_OF_FEATURE
```

Open a pull request against the main python-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 explains 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).
26 changes: 26 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
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

.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
Empty file.
29 changes: 29 additions & 0 deletions open_feature/immutable_dict/mapping_proxy_type.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
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
41 changes: 22 additions & 19 deletions open_feature/open_feature_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,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]
Expand Down Expand Up @@ -236,6 +235,9 @@ def evaluate_flag_details(
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,
flag_type=flag_type,
Expand All @@ -250,24 +252,19 @@ 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
)
reversed_merged_hooks = merged_hooks[:]
reversed_merged_hooks.sort()

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

Expand All @@ -284,25 +281,31 @@ def evaluate_flag_details(
)

after_hooks(
flag_type, hook_context, flag_evaluation, reversed_merged_hooks, None
flag_type,
hook_context,
flag_evaluation,
reversed_merged_hooks,
hook_hints,
)
tcarrio marked this conversation as resolved.
Show resolved Hide resolved

return flag_evaluation

except OpenFeatureError as e:
error_hooks(flag_type, hook_context, e, reversed_merged_hooks, None)
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, None)
error_message = getattr(e, "error_message", str(e))
except Exception as err: # noqa
error_hooks(flag_type, hook_context, err, reversed_merged_hooks, hook_hints)

error_message = getattr(err, "error_message", str(err))
return FlagEvaluationDetails(
flag_key=flag_key,
value=default_value,
Expand All @@ -312,7 +315,7 @@ def evaluate_flag_details(
)

finally:
after_all_hooks(flag_type, hook_context, reversed_merged_hooks, None)
after_all_hooks(flag_type, hook_context, reversed_merged_hooks, hook_hints)

def _create_provider_evaluation(
self,
Expand Down
4 changes: 4 additions & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,7 @@ Thanks so much to our contributors.
</a>

Made with [contrib.rocks](https://contrib.rocks).

### Development

If you would like to contribute to the project, please see our [contributing](./CONTRIBUTING.md) documentation!
25 changes: 21 additions & 4 deletions tests/hooks/test_hook_support.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -7,26 +9,33 @@
before_hooks,
error_hooks,
)
from open_feature.immutable_dict.mapping_proxy_type import MappingProxyType


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=hook_context, exception=ANY, hints=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.before.assert_called_with(hook_context=hook_context, hints=hook_hints)


def test_after_hooks_run_after_method(mock_hook):
Expand All @@ -35,20 +44,28 @@ 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.after.assert_called_with(
hook_context=hook_context, details=flag_evaluation_details, hints=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.finally_after.assert_called_with(
hook_context=hook_context, hints=hook_hints
)