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: Add Flipt provider #143

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

atmask
Copy link
Contributor

@atmask atmask commented Jan 3, 2025

Overview

This PR implements the Flipt provider by wrapping around the OFREP Provider and leveraging Flipt's OFREP api

Discussion Points:

  • Should we support passing the namespace in the constructor this way or just leave it up to users to do it themselves via the generic headers_factory (like they would need to for authentication)

Related Issues

Fixes #89

How to test

Unit Tests: From the base of the package run hatch test

Local Usage: Go to the base of the new packager (i.e. provider/openfeature-provider-flipt), run pip install -e ., and then open a python repl with python3:

from openfeature import api
from openfeature.contrib.provider.flipt import FliptProvider
api.set_provider(FliptProvider(base_url="<flipt instance>", namespace="<your namespace>"))
client = api.get_client()
client.get_boolean_value("<your feature flag key>", True)

@atmask atmask requested a review from a team as a code owner January 3, 2025 21:05
@atmask atmask changed the title [Issue 89] Add Flipt provider feat: Add Flipt provider Jan 3, 2025
@beeme1mr beeme1mr requested a review from markphelps January 3, 2025 21:39
Copy link

@markphelps markphelps left a comment

Choose a reason for hiding this comment

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

lgtm! just curious if this gives end users much over the generic Python OFREP impl? Is the end goal of OFREP to make it easier to implement provider specific sdks or to be 'the' SDK for all providers that support it?

cc @beeme1mr

Comment on lines +46 to +52
if headers_factory is None:
headers = {"X-Flipt-Namespace": namespace}
else:
headers = {
**headers_factory(),
"X-Flipt-Namespace": namespace,
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Something we might want to consider is that this will result in the output of the user's header's factory being called once and the output being merged into the dict here. That dict will always be returned without the user's header's factory being called again. This could be fine it you are okay with it but it is a change from the current implementation where the user's headers_factory is called on every feature flag evaluation in the OFREP provider.

I think I could turn this into a closure around the user's headers_factory callable if that is something we might be concerned about.

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.

Add support for Flipt provider
2 participants