-
Notifications
You must be signed in to change notification settings - Fork 1k
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
docs: klip-41: ksqlDB .Net Client #6613
Conversation
@confluentinc It looks like @alex-basiuk just signed our Contributor License Agreement. 👍 Always at your service, clabot |
Thanks @alex-basiuk - this is really exciting stuff. We haven't really come up with a good way to structure development around ksqlDB clients. Did you have any idea about where you were planning on having the code live? Do you have a prototype or code hosted somewhere publicly that you want us to review? A good way forward on this (the KLIP LGTM) would be to start development as an independent client and then when it gains traction popularity, we can bring it under the ksqlDB "official" umbrella of clients and in the meantime, we can point to it from our README to help get it some traction. |
Thanks @agavra, it's not publicly available yet, but will be soon. I'll post updates here and will add a link to the README. |
## What is in scope | ||
|
||
* A new client implemented in C#. | ||
* Initially, the client will be targeted at .NET Core 3 and .NET Framework 5. Support of older versions will be added later. |
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'm not versed in .NET, but one thing I'm wondering is how support for multiple versions will look over time. Any ideas about how to manage that workflow?
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.
@MichaelDrogalis .NET 5 and .NET Core 3 are very close in terms of APIs. I don't think .NET 5 has any breaking changes in the libraries required by this project. It won't be hard to support both until end of support of .NET Core 3.1. Basically, you can have one project which targets multiple framework versions.
Support of older versions is probably more involved.
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.
Cool, that makes sense. Mostly just asking since Im unfamiliar with the .NET stack.
such as list and describe. | ||
* The client will utilise C# [async streams](https://docs.microsoft.com/en-us/dotnet/csharp/tutorials/generate-consume-asynchronous-stream) feature. | ||
* The client will also support a direct / synchronous interaction style. | ||
* High-quality documentation and examples. |
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.
Super. :) If this is going to live inside of a repo owned by you for a while, we can put the documentation in a README there so it is self-contained.
## Test plan | ||
|
||
Unit and integration test will be mandatory. Performance benchmarks are desirable but unlikely be the first priority. | ||
Ideally, tests should be executed on both Windows and Linux platforms. |
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.
Do any CI providers make dual Windows tests easy? Been a while since I've tested much on Windows.
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.
@MichaelDrogalis Travis, CircleCI, Azure CI. Folks from confluent-kafka-dotnet use Travis.
cc @vcrfxia might have some input on this, I'm not totally sure what our versioning strategy for the client is going forward |
ksql/ksqldb-rest-app/src/main/java/io/confluent/ksql/api/server/ServerVerticle.java Lines 344 to 346 in e6ba436
Currently the latest (and only) version is application/vnd.ksql.v1+json : ksql/ksqldb-rest-model/src/main/java/io/confluent/ksql/rest/entity/KsqlMediaType.java Line 31 in e6ba436
|
Thanks @vcrfxia Also, I noticed that the |
The decision to include this option predates me. Perhaps @rodesai has context?
Thanks for the feedback. The field comes from ksqlDB's use of Jackson subtypes for server response types. ksqlDB 1.0 will likely include pretty significant changes to the server API endpoints. That would be the time to reconsider this, as removing an existing field would be a breaking change. |
Description
A proposal for a .Net client
Testing done
N/A
Reviewer checklist