-
Notifications
You must be signed in to change notification settings - Fork 71
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: introduce grpc sync for flagd #297
feat: introduce grpc sync for flagd #297
Conversation
7458bb6
to
4804147
Compare
00e5c19
to
9e10b84
Compare
9e10b84
to
b83df93
Compare
That's amazing! |
I'm going to give this a try and a thorough review on Monday! |
Works for me! I couple things I noted in testing:
|
6797d9b
to
765f431
Compare
Yeah, unfortunately, that's the downside of grpc. We need the server impl and currently, it's my toy server 😿 And the reason is simple, unlike file, crds, the server impl can be anything. It could be triggered from DB change, github workflow or anything. But maybe I can expand my server with few more use-cases 🤔 I addressed other concerns in respective comments :) |
## This PR Is related to open-feature/ofep#45 & open-feature/flagd#297 Introduce the service contract of grpc sync. Consider checking my personal buf repository - https://buf.build/kavindudodan/flagd for a preview --------- Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com> Signed-off-by: Kavindu Dodanduwa <Kavindu-Dodan@users.noreply.github.com> Co-authored-by: Skye Gill <gill.skye95@gmail.com> Co-authored-by: Todd Baert <toddbaert@gmail.com>
4bd7848
to
5058b62
Compare
@toddbaert I have added connection retry logic with the latest change set :) if you can do another round of review, and make sure this fits flagd standard, thne I will convert this draft to a normal PR 🙌 |
0d5a588
to
8afee0e
Compare
b9e5319
to
7230abc
Compare
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Co-authored-by: James Milligan <75740990+james-milligan@users.noreply.github.com> Signed-off-by: Kavindu Dodanduwa <Kavindu-Dodan@users.noreply.github.com>
Co-authored-by: James Milligan <75740990+james-milligan@users.noreply.github.com> Signed-off-by: Kavindu Dodanduwa <Kavindu-Dodan@users.noreply.github.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Co-authored-by: James Milligan <75740990+james-milligan@users.noreply.github.com> Signed-off-by: Kavindu Dodanduwa <Kavindu-Dodan@users.noreply.github.com>
Co-authored-by: James Milligan <75740990+james-milligan@users.noreply.github.com> Signed-off-by: Kavindu Dodanduwa <Kavindu-Dodan@users.noreply.github.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
12cc44f
to
02e8433
Compare
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.
Minor concerns, nice work :)
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Co-authored-by: Skye Gill <gill.skye95@gmail.com> Signed-off-by: Kavindu Dodanduwa <Kavindu-Dodan@users.noreply.github.com>
Co-authored-by: Skye Gill <gill.skye95@gmail.com> Signed-off-by: Kavindu Dodanduwa <Kavindu-Dodan@users.noreply.github.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
🤖 I have created a release *beep* *boop* --- ## [0.4.0](v0.3.7...v0.4.0) (2023-03-02) ### ⚠ BREAKING CHANGES * Use OTel to export metrics (metric name changes) ([#419](#419)) ### 🧹 Chore * add additional sections to the release notes ([#449](#449)) ([798f71a](798f71a)) * attach image sbom to release artefacts ([#407](#407)) ([fb4ee50](fb4ee50)) * **deps:** update actions/configure-pages digest to fc89b04 ([#417](#417)) ([04014e7](04014e7)) * **deps:** update amannn/action-semantic-pull-request digest to b6bca70 ([#441](#441)) ([ce0ebe1](ce0ebe1)) * **deps:** update docker/login-action digest to ec9cdf0 ([#437](#437)) ([2650670](2650670)) * **deps:** update docker/metadata-action digest to 3343011 ([#438](#438)) ([e7ebf32](e7ebf32)) * **deps:** update github/codeql-action digest to 32dc499 ([#439](#439)) ([f91d91b](f91d91b)) * **deps:** update google-github-actions/release-please-action digest to d3c71f9 ([#406](#406)) ([6e1ffb2](6e1ffb2)) * disable caching tests in CI ([#442](#442)) ([28a35f6](28a35f6)) * fix race condition on init read ([#409](#409)) ([0c9eb23](0c9eb23)) * integration test stability ([#432](#432)) ([5a6a5d5](5a6a5d5)) * integration tests ([#312](#312)) ([6192ac8](6192ac8)) * reorder release note sections ([df7bfce](df7bfce)) * use -short flag in benchmark tests ([#431](#431)) ([e68a6aa](e68a6aa)) ### 🐛 Bug Fixes * **deps:** update kubernetes packages to v0.26.2 ([#450](#450)) ([2885227](2885227)) * **deps:** update module github.com/bufbuild/connect-go to v1.5.2 ([#416](#416)) ([feb7f04](feb7f04)) * **deps:** update module github.com/open-feature/go-sdk-contrib/providers/flagd to v0.1.9 ([#427](#427)) ([42d2705](42d2705)) * **deps:** update module github.com/open-feature/open-feature-operator to v0.2.29 ([#429](#429)) ([b7fae81](b7fae81)) * **deps:** update module github.com/stretchr/testify to v1.8.2 ([#440](#440)) ([ab3e674](ab3e674)) * **deps:** update module golang.org/x/net to v0.7.0 ([#410](#410)) ([c6133b6](c6133b6)) * **deps:** update module sigs.k8s.io/controller-runtime to v0.14.5 ([#454](#454)) ([f907f11](f907f11)) * remove non-error error log from parseFractionalEvaluationData ([#446](#446)) ([34aca79](34aca79)) ### ✨ New Features * add debug logging for merge behaviour ([#456](#456)) ([dc71e84](dc71e84)) * add Health and Readiness probes ([#418](#418)) ([7f2358c](7f2358c)) * Add version to startup message ([#430](#430)) ([8daf613](8daf613)) * introduce flag merge behaviour ([#414](#414)) ([524f65e](524f65e)) * introduce grpc sync for flagd ([#297](#297)) ([33413f2](33413f2)) * refactor and improve K8s sync provider ([#443](#443)) ([4c03bfc](4c03bfc)) * Use OTel to export metrics (metric name changes) ([#419](#419)) ([eb3982a](eb3982a)) ### 📚 Documentation * add .net flagd provider ([73d7840](73d7840)) * configuration merge docs ([#455](#455)) ([6cb66b1](6cb66b1)) * documentation for creating a provider ([#413](#413)) ([d0c099d](d0c099d)) * updated filepaths for schema store regex ([#344](#344)) ([2d0e9d9](2d0e9d9)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This PR
Attempts to resolve #249 by introducing grpc sync provider to flagd.
OFEP [approved] - https://github.com/open-feature/ofep/blob/main/OFEP-flagd-grpc-sync.md
How to test/run ?
Flagd acts as the grpc client, hence you need at least a minimal mock server. For this, you can utilize this [1] server implementation.
Startup arguments of flagd now support grpc target uri. This can be provided with
grpc://
, for example,./flagd start --uri grpc://localhost:8090
Technical highlights
What is not included (follow up improvements)
[1] - https://github.com/Kavindu-Dodan/flagd-grpc-sync-server
[2] - https://buf.build/open-feature/flagd