-
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: Add support for unix socket path and secure connection #8
Conversation
Signed-off-by: Jose Colella <jose.colella@gusto.com>
Signed-off-by: Jose Colella <jose.colella@gusto.com>
- Documentation on how to connect via unix sockets and secure connection Signed-off-by: Jose Colella <jose.colella@gusto.com>
providers/openfeature-flagd-provider/lib/openfeature/flagd/provider/client.rb
Outdated
Show resolved
Hide resolved
Co-authored-by: Skye Gill <gill.skye95@gmail.com> Signed-off-by: Jose Miguel Colella <josecolella@yahoo.com>
…vider/client.rb Co-authored-by: Skye Gill <gill.skye95@gmail.com> Signed-off-by: Jose Miguel Colella <josecolella@yahoo.com>
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.
Since this is integrating with another service, I think it'd be extremely helpful/important to be able to validate that it is able to connect and interact with it.
You mention being able to locally with docker. I'd suggest making a docker-compose.yml
to be able to bring that up. With that, you'd be able to add some RSpec hooks for making sure it's running. For example:
RSpec.configure do |config|
next if config.dry_run
config.when_first_matching_example_defined(type: :system) do
# Run FlagD for integration specs
unless config.dry_run
# code to start flagd here
end
end
You'd need to either add :integration
to the specs, or configure an RSpec to infer that based on a path, ie spec/integration. I'm not positive how that is done, as my initial searching seems to be tied to rspec-rails specifically.
@technicalpickles One of the things we've done in the other SDK repos is implement this integration test suite, which relies on flagd (running in a container in CI), and this flagd provider to test basic SDK functionality. Basically, it runs exactly what you propose, but to validate the SDK behavior end-to-end, including provider registration and the flag evaluation API. We could run the same integration suite here, or just a subset. |
@technicalpickles Was there anything else you saw for this PR that needed modification? |
I see some changes around the config/ENV/default config merging, and I also see a (previously) skipped test here that might be impacted. |
Co-authored-by: Todd Baert <toddbaert@gmail.com> Signed-off-by: Jose Miguel Colella <josecolella@yahoo.com>
As we discussed in the CNCF slack, it seems like we can make the cert path optional, since Ruby will otherwise use system certs. |
providers/openfeature-flagd-provider/lib/openfeature/flagd/provider/client.rb
Outdated
Show resolved
Hide resolved
providers/openfeature-flagd-provider/lib/openfeature/flagd/provider/client.rb
Outdated
Show resolved
Hide resolved
providers/openfeature-flagd-provider/lib/openfeature/flagd/provider/configuration.rb
Show resolved
Hide resolved
…vider/client.rb Co-authored-by: Josh Nichols <josh@technicalpickles.com> Signed-off-by: Jose Miguel Colella <josecolella@yahoo.com>
…vider/client.rb Co-authored-by: Josh Nichols <josh@technicalpickles.com> Signed-off-by: Jose Miguel Colella <josecolella@yahoo.com>
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.
Chatted with @josecolella, and the comments I've left can be followup improvements. So approving ✅
🤖 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 <109696520+openfeaturebot@users.noreply.github.com>
This PR
Related Issues
Resolves #5
Resolves #4
Notes
Follow-up Tasks
How to test
@beeme1mr @toddbaert How would I test unix socket path locally. I'm using
for flagd testing