EventStore / EventStoreDB-Client-Java

Official Asynchronous Java 8+ Client Library for EventStoreDB 20.6+
https://eventstore.com
Apache License 2.0
61 stars 19 forks source link

ConnectionState.connect() should use generic Grpc.newChannelBuilderForAddress() #249

Open coreydowning opened 8 months ago

coreydowning commented 8 months ago

EventStoreDB-Client Version: 5.1.0

The ConnectionState.connect(...) method currently creates a managed gRPC channel using the NettyChannelBuilder from io.grpc:grpc-netty-shaded. We have a project that uses gRPC for multiple things and needs to use the io.grpc:grpc-netty transport instead of the shaded one. Because the connect method uses the netty shaded channel builder, this is impossible.

My suggested change is to move to using one of the flavors of Grpc.newChannelBuilder() function, allowing gRPC to dynamically choose the correct transport based on what is available on the classpath.

YoEight commented 8 months ago

Hey @coreydowning, I wasn't aware of that. I'll have that patch sorted out as soon as possible

coreydowning commented 8 months ago

Thank you very much! I sorted out most of it but haven't figured out an alternative to this

if (!settings.isTlsVerifyCert())
                    sslContextBuilder.trustManager(InsecureTrustManagerFactory.INSTANCE);
YoEight commented 8 months ago

So far it seems impossible to keep feature parity while being grpc-backend agnostic. I can replicate everything but being able to disable certificate verification, which is quite useful in testing/dev environments. I'm open to suggestion, the related PR is linked to this issue.

coreydowning commented 8 months ago

I will take a look at this today and see if I can come up with anything.

michael-schnell commented 7 months ago

Good idea to decouple the stuff! The GRPC client currently does not work with Quarkus native (GraalVM) because of multiple problems caused by the grpc-netty-shaded implementation. It would be great if it would be possible to use the Quarkus GRPC client instead.

michael-schnell commented 2 months ago

Any update?

YoEight commented 1 month ago

I haven't looked at that PR since my last message. Did grpc-stub change during this time? This is not a priority, only best effort.