feat: add Flipt provider #178
Conversation
dmitryrogov
force-pushed
the
feat/flipt-provider
branch
from
023464b
to
6c258a4
Compare
github-actions
bot
requested review from
matthewelwell,
thomaspoignant and
vpetrusevici
Signed-off-by: Jens Henneberg <[email protected]> Signed-off-by: Rogov Dmitry <[email protected]>
Signed-off-by: Rogov Dmitry <[email protected]>
Signed-off-by: Michael Beemer <[email protected]> Signed-off-by: Rogov Dmitry <[email protected]>
Signed-off-by: Rogov Dmitry <[email protected]>
dmitryrogov
force-pushed
the
feat/flipt-provider
branch
from
ca5848b
to
b9fb281
Compare
beeme1mr
requested review from
toddbaert
and removed request for
matthewelwell,
thomaspoignant and
vpetrusevici
|
Hey @dmitryrogov, thanks for the PR. @toddbaert, @askpt, or @markphelps could someone please review this PR? |
| /* Or create config directly: | ||
| var config = new FliptProviderConfiguration | ||
| { | ||
| ServiceUri = new Uri("http://localhost:9000"), |
There was a problem hiding this comment.
I think we'll want to support authentication as well, likely with some helpers that can add either clientToken authentication (https://docs.flipt.io/authentication/using-tokens) or JWT authentication (https://docs.flipt.io/authentication/using-jwts)
The only difference really is the value of the Authorization header that is sent to Flipt:
- Client Token:
Authentication: Bearer {token} - JWT:
Authentication: JWT {token}
There was a problem hiding this comment.
also support for our authentication methods still needs to be added @dmitryrogov
| { | ||
| _configuration = configuration; | ||
| var channel = GrpcChannel.ForAddress(configuration.ServiceUri); | ||
| _client = new EvaluationServiceClient(channel); |
There was a problem hiding this comment.
we may want to think about making a HTTP client and making that the default as GRPC requires HTTP2 and might not work in all environments.
I realize making an HTTP C# client for Flipt is not a small undertaking though, we've had this issue open for a bit in our server-side SDK monorepo: flipt-io/flipt-server-sdks#86
There was a problem hiding this comment.
That's true. It would be great to use a client interface instead of the implementation itself (GRPC client SDK, HTTP client SDK or server SDK), but the library doesn't support that at the moment.
For now I can I suggest either modifying the repository flipt-grpc-dotnet by adding a GRPC client interface, or making this constructor public
There was a problem hiding this comment.
Yeah we actually do something similar in our Go Open Feature provider, which supports both GRPC and HTTP
There its all based on the protocol in the address / URL string for which transport to use: https://github.com/open-feature/go-sdk-contrib/tree/main/providers/flipt#configuration
We could do something similar here, where at first we would only support the GRPC transport like you have done, but once we ideally have a C# REST/HTTP SDK then we can add support here in a non-breaking way, by checking the protocol.
There was a problem hiding this comment.
I think this still needs addressing @dmitryrogov
|
Sorry I've been slow on this one. I'll get to it first thing Monday. |
| @@ -0,0 +1 @@ | |||
| 0.0.1 | |||
There was a problem hiding this comment.
Also add an entry in the release-plz-manifest with a version number matching this, or it will release the first version as 1.0.0.
There was a problem hiding this comment.
Sorry this took so long, this looks good to me, and I really tried to find something wrong. If @markphelps has no objections I'm OK with merging. We can also add it to the ecosystem.
One important note about this, though.
| /* Or create config directly: | ||
| var config = new FliptProviderConfiguration | ||
| { | ||
| ServiceUri = new Uri("http://localhost:9000"), |
There was a problem hiding this comment.
also support for our authentication methods still needs to be added @dmitryrogov
| { | ||
| _configuration = configuration; | ||
| var channel = GrpcChannel.ForAddress(configuration.ServiceUri); | ||
| _client = new EvaluationServiceClient(channel); |
There was a problem hiding this comment.
I think this still needs addressing @dmitryrogov
| ``` | ||
|
|
||
| ## Boolean flag evaluation | ||
| To perform Boolean flag evaluation, such as `GetBooleanValue` or `GetBooleanDetail` you can utilize [Boolean Evaluation](https://docs.flipt.io/reference/evaluation/boolean-evaluation) or [Variant Evaluation](https://docs.flipt.io/reference/evaluation/variant-evaluation) APIs. |
There was a problem hiding this comment.
this seems a bit odd, wouldnt we want to rely on boolean flags always for GetBooleanX calls ?
This PR
Adding Flipt provider integration.
How to test
Run Flipt feature server locally
docker run -d -p 8080:8080 -p 9000:9000 -v $HOME/flipt:/var/opt/flipt docker.flipt.io/flipt/flipt:latest