search

cyanfish / grpc-dotnet-namedpipes

Named pipe transport for gRPC in C#/.NET
Apache License 2.0
190 stars 48 forks source link

Upgrading to GRPC V1.32.0 breaks namedpipes #13

Closed DanFTRX closed 3 years ago

DanFTRX commented 3 years ago

Upgrading from an earlier version to v1.32+ breaks this package. It seems to be due to the serializer switch in this change.

ClientConnectionContext(and possibly more) is still using the older deprecated serializer (see https://github.com/cyanfish/grpc-dotnet-namedpipes/blob/master/GrpcDotNetNamedPipes/Internal/ClientConnectionContext.cs#L69 )

cyanfish commented 3 years ago

Thanks, I'll see if I can find time to fix this, also happy to accept PRs.

Probably the best fix is to publish two new versions, 1.1.2 that restricts the gRPC dependency, and 1.2.0 that uses the new serializer.

DanFTRX commented 3 years ago

I gave it a look, it seems you need to create a SerializationContext and DeserializationContext class for yourself and use it for the serialization/deserialization. The code changes outside of creating those context classes seem pretty minimal. I looked into the GRPC implementations: https://github.com/grpc/grpc/blob/master/src/csharp/Grpc.Core/Internal/DefaultSerializationContext.cs and https://github.com/grpc/grpc/blob/master/src/csharp/Grpc.Core/Internal/DefaultDeserializationContext.cs used at https://github.com/grpc/grpc/blob/master/src/csharp/Grpc.Core/Internal/AsyncCall.cs#L98 and https://github.com/grpc/grpc/blob/master/src/csharp/Grpc.Core/Internal/AsyncCall.cs#L163 and others. At an unfamiliar glance, it may be that all you need to do is copy those classes(as they are internal and can't be referenced), and they seem pretty simple. However, given the apparent need for ThreadLocal, and my unfamiliarity with the GRPC internals to know why, I wouldn't feel great writing a PR because I don't like writing code I don't fully understand, and I don't have the time to look into it much further.

ElDuderinoBerlin commented 3 years ago

Where is the update?

a-jaeger commented 3 years ago

Updating the NuGet-Package "Google.Protobuf" to Version 3.14.0 should do the thing.

A Workaround for all using this NuGet-Package ist to first include the NuGet-Package "Google.Protobuf v3.14.0" and afterwards the GrpcDotNetNamePipes, because the Reference to the Google.Protobuf is defined as >=3.11.2.

ElDuderinoBerlin commented 3 years ago

@a-jaeger: did what you suggested, but still isn't working!

DanFTRX commented 3 years ago

Your only option is to use an older version of gRPC until an update fixes this Sounds like @cyanfish is busy, understandable. Hopefully someone else with the time/motivation to look into this further will pick this up and submit a PR.

cyanfish commented 3 years ago

Fixed in version 1.2.0.