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: [WIP] Add the open feature flipt client. Tests still required #179

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

Conversation

atmask
Copy link
Contributor

@atmask atmask commented Feb 5, 2025

This PR

Background:
This PR adds a new provider to support flipt-client evaluation. This provider more flexibility to end users looking to consume from the Flipt and it allows OpenFeature to be used directly alongside Flipt Cloud which only supports the Client side sdk atm (self-hosted supports both client and server side sdks).

Implementation

Related Issues

#163

Follow-up Tasks

NOTE: Tests are still WIP

  • Release the package
  • Update Flipt's OpenFeature support webpage
  • Update OpenFeature's ecosystem webpage

@atmask atmask requested a review from a team as a code owner February 5, 2025 04:15
Copy link

codecov bot commented Feb 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.39%. Comparing base (ed121bd) to head (003b3fb).
Report is 17 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #179   +/-   ##
=======================================
  Coverage   94.39%   94.39%           
=======================================
  Files          14       14           
  Lines         749      749           
=======================================
  Hits          707      707           
  Misses         42       42           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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 so far! one comment about adding a potential compatability note in the readme

thank you @atmask !!


```


Choose a reason for hiding this comment

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

we may want to put a note in the README that this provider will only work on platforms that use Glibc as the C library. Also it will only work for the OS'es mentioned here because we are using FFI under the hood for the evaluation engine clientside: https://github.com/flipt-io/flipt-client-sdks?tab=readme-ov-file#ffi


def get_metadata(self) -> Metadata:
return Metadata(
name='OpenFeature Flipt Client-Side Provider',
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
name='OpenFeature Flipt Client-Side Provider',
name='Flipt',

I'd recommend keeping this as short as possible since it may be used in telemetry and logs.

value=default_value,
reason=Reason.ERROR,
error_message=str(e),
error_code=None ## Explicitly set to None. No clear mapping atm from Flipt error to OpenFeature error codes
Copy link
Member

Choose a reason for hiding this comment

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

Please use the General error code if there isn't a more appropriate one.

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.

3 participants