-
Notifications
You must be signed in to change notification settings - Fork 3
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: integrate flagd provider with OpenFeature SDK #18
Conversation
Signed-off-by: Alexandre Chakroun <[email protected]>
Signed-off-by: Alexandre Chakroun <[email protected]>
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.
No qualms with how this is written 🦑
To answer your thoughts in the PR, we do not differentiate between floats and integers on the flag eval interface, but I think we should (added an issue for it).
In the MetaProvider I'm working on, I chose to take a dependency on OpenFeature to access the Metadata
, ResolutionDetails
and pseudo-enumerations defined there. While it does lock to a particular version of OF, I think having those defined upstream makes more sense than us redefinining them in the contrib providers.
providers/openfeature-flagd-provider/lib/openfeature/flagd/provider/client.rb
Outdated
Show resolved
Hide resolved
Signed-off-by: Alexandre Chakroun <[email protected]>
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.
Nice, thanks.
🤖 I have created a release *beep* *boop* --- ## [0.1.0](openfeature-flagd-provider-v0.0.1...openfeature-flagd-provider/v0.1.0) (2024-05-16) ### ⚠ BREAKING CHANGES * update flagd name and grpc schema ([#30](#30)) ### ✨ New Features * Add flagd provider ([#2](#2)) ([98b695b](98b695b)) * Add support for unix socket path and secure connection ([#8](#8)) ([88436c7](88436c7)) * Flagd provider uses structs from sdk ([#24](#24)) ([d437e7f](d437e7f)) * integrate flagd provider with OpenFeature SDK ([#18](#18)) ([80d6d02](80d6d02)) * Return default value on error ([#25](#25)) ([f365c6d](f365c6d)) * update flagd name and grpc schema ([#30](#30)) ([ddd438a](ddd438a)) ### 🧹 Chore * Format with standard ([#20](#20)) ([bf25043](bf25043)) * Make things work ([#13](#13)) ([5968037](5968037)) * update link to use new doc domain ([#12](#12)) ([9baff65](9baff65)) * upgrade grpc client ([#16](#16)) ([23ed78a](23ed78a)) ### 🔄 Refactoring * OpenFeature::FlagD::Provider::Configuration ([#14](#14)) ([3686eb5](3686eb5)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Signed-off-by: OpenFeature Bot <[email protected]>
This PR
Notes
Metadata
andResolutionDetails
structs be defined on the Open Feature SDK directly and used by providers?fetch_integer_value
andfetch_float_value
instead offetch_number_value
? I did some hack by testing the type of the default value but I feel it would make more sense for the provider to default to using number instead of integer or float if the distinction is not supported