Skip to content
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

Switch to gogo and yarpc compilers #4040

Conversation

vytautas-karpavicius
Copy link
Contributor

@vytautas-karpavicius vytautas-karpavicius commented Mar 8, 2021

It seems that previous switch to use google/protobuf 1.4.x (v2) compiler was made too soon. This is due to the fact, that in Cadence we use YARPC framework which is still on gogo. Messages in gogo are not compatible with 1.4.x protobuf library and causes panics during serialization. YARPC is in the process of migrating off gogo. But as I was told it is at least several months away.

In order for GRPC migration to move forward, we will switch back to gogo. However this is not a pure revert of previous PR. We still want to get rid of all gogo extensions. Having them will make future switch to protobuf 1.4.x more difficult.

To generate YARPC server and client code protoc-gen-yarpc-go plugin is used. Currently used version does not support alias in go_package option however. A fix was made for it. Since it is not released yet, I have added a temporary replace in go.mod for it. Another replace is necessary for apache/thrift. Updated yarpc module brings new version of apache/thrift with breaking changes. However tchannel-go and ringpop-go still uses older one.

@coveralls
Copy link

coveralls commented Mar 8, 2021

Coverage Status

Coverage increased (+0.05%) to 66.845% when pulling e0b17fa on vytautas-karpavicius:protoc-gogo-yarpc into 1de8cd4 on uber:master.

@vytautas-karpavicius vytautas-karpavicius force-pushed the protoc-gogo-yarpc branch 5 times, most recently from 1b04c03 to 5aed510 Compare March 9, 2021 11:15
@vytautas-karpavicius vytautas-karpavicius marked this pull request as ready for review March 9, 2021 13:28
@vytautas-karpavicius vytautas-karpavicius merged commit 764500a into cadence-workflow:master Mar 15, 2021
yux0 pushed a commit to yux0/cadence that referenced this pull request May 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants