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

fix: Revise ConfigCat provider #280

Merged
merged 4 commits into from
Sep 17, 2024

Conversation

adams85
Copy link
Contributor

@adams85 adams85 commented Sep 17, 2024

This PR

As part of our official support for ConfigCat OpenFeatures providers, I revised the .NET provider and am proposing the following improvements and corrections:

  • Let's implement InitializeAsync (so feature flag evaluation after awaiting SetProviderAsync will certainly succeed) and ShutdownAsync (so the underlying client is disposed, i.e. the polling loop is stopped as soon as it's not needed any more).
  • UserBuilder made some unnecessary heap memory allocations (e.g. when converting attribute keys to upper case). Let's eliminate these. (As feature flag evaluation can occur on hot paths, we should aim to minimize allocations to put as little pressure on the GC as possible.)
  • When id or identifier is not specified in the evaluation context, a random user id is generated (a guid). I advise against this as it can lead to confusing user experience for such contexts, especially in the case of percentage option rules (which are based on user id by default). I suggest some constant string like "<n/a>" for such cases.
  • An "end-to-end" test case that tests the provider in a "real life" situation (i.e. using the provider via SetProviderAsync) and exercises all of its parts (evaluation with context, user attribute translation, user object building, etc).
  • Upgrade to the latest ConfigCat SDK version (this is necessary for the new test case).
  • Corrections to README.md, especially to code samples which seem to be outdated or even contain syntax errors.

Adam Simon added 4 commits September 17, 2024 19:30
…ProviderAsync works) and ShutdownAsync (so the underlying client is disposed)

Signed-off-by: Adam Simon <[email protected]>
…UserBuilder + make evaluation deterministic for contexts where user id is not specified

Signed-off-by: Adam Simon <[email protected]>
@beeme1mr beeme1mr merged commit 0b2d5f2 into open-feature:main Sep 17, 2024
10 checks passed
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