-
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
Make GrpcStreamBroadcaster
compatible with both grpcio
and grpclib
#49
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Updated. Will fix up before merging. |
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.
LGTM, it only needs fixing up as stated...
Updated. |
Enabled auto-merge. |
The good news is the merge queue checks now works correctly, the bad news is I forgot 2 fixes need to be applied to fix the CI, so this PR depends now on this PR to be merged: |
The merge-base changed after approval.
This is a preparation commit to make the client base library compatible with both `grpcio` and `grpclib`. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
This module will provide a way to deal with multiple grpc libraries. For now it only handles exceptions from both libraries, but it can be extended to handle other differences in the future. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
With this change `GrpcStreamBroadcaster` can be used with both `grpcio` and `grpclib` implementations of gRPC. This is achieved by using `collections.abc.AsyncIterator` instead of `grpc.aio.UnaryStreamCall` in the `stream_method` parameter of `GrpcStreamBroadcaster` to make it more generic, and catching exceptions from both `grpcio` and `grpclib` in the `async for` loop. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Rebased, it needs a new approval. |
With this change
GrpcStreamBroadcaster
can be used with bothgrpcio
andgrpclib
implementations of gRPC.This is achieved by using
AsyncIterator
instead ofgrpc.aio.UnaryStreamCall
in thestream_method
parameter ofGrpcStreamBroadcaster
to make it more generic, and catching exceptions from bothgrpcio
andgrpclib
in theasync for
loop.To deal with any of the libraries being installed, a new module is added to handles exceptions from both libraries, by conditionally importing the exception and provide a dummy one if the library is not installed.
Now both
grpcio
andgrpclib
are optional, and can be installed usingfrequenz-client-base[grpcio]
orfrequenz-client-base[grpclib]
respectively.We also add some tests for
GrpcStreamBroadcaster
.