Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: In-memory provider for e2e testing and minimal usage #546
feat: In-memory provider for e2e testing and minimal usage #546
Changes from 1 commit
c69b5a9
29c99eb
9af7d1d
4f90f66
47d8127
542395e
4cf9ba4
f85cbb5
1e508ca
071fa03
41987b8
9c67eeb
3cb8ee1
4d9ceed
7f86107
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unused and unnecessary 🤔 Was the intention here to fail fast with flag configuration validation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leftover, will remove it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I propose adding a context evaluator backed by a user-provided callback
See go-sdk implementation for example - https://github.com/open-feature/go-sdk/blob/main/pkg/openfeature/memprovider/in_memory_provider.go#L167-L175
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trying to understand what is the purpose and value here with enhancing with additional testing capabilities. The in-memory provider is for testing purposes, how is adding a context evaluator helping with testing actual flow ? it will only test the testing provider ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current implementation resides as a
test
source. This mismatch with the requirement of the specification.When we agreed on the in-memory provider through OFEP and added it to the specification 1, the agreement was to make the in-memory an extra provider built into the SDK itself. And we agreed to support evaluation contexts through lambda/callback.
So the packaging of the implementation should be corrected first. I am proposing to move it to a package named
memprovider
or any better-named package.Regarding the purpose, the main benefit of having evaluation context support is testing SDK. It allows us to write end-to-end tests (or to migrate existing ones based on flagd) for context evaluations and verify SDK correctness. Besides, end users can use the provider to prototype OpenFeature features.
Footnotes
https://github.com/open-feature/spec/blob/main/specification/appendix-A.md#in-memory-provider ↩