uber-go / cadence-client

Framework for authoring workflows and activities running on top of the Cadence orchestration engine.
https://cadenceworkflow.io
MIT License
350 stars 131 forks source link

Added 2-way proto-thrift mapper #1130

Closed vytautas-karpavicius closed 3 years ago

vytautas-karpavicius commented 3 years ago

What changed? Up to this point, there was one way mapping for thrift to proto types. This was used to create a compatibility layer, such that existing client could communicate in gRPC with server.

Now adding mapping in other direction as well. This will be used to convert internal of client itself to use proto types, but still accept publicly exposed thrift types.

Why? To migrate internals of go client to use proto types.

Only publicly exposed API surface will keep thrift for compatibility.

How did you test it? Added unit tests that ensures the following invariant hold true:

assert.Equal(t, item, proto.TYPE_NAME(thrift.TYPE_NAME(item)))

Potential risks New mappers not used yet.

coveralls commented 3 years ago

Pull Request Test Coverage Report for Build dde8d9ef-8a19-406a-9811-46bbaacfd9b5


Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/compatibility/thrift/helpers.go 0 15 0.0%
internal/compatibility/proto/helpers.go 17 36 47.22%
internal/compatibility/proto/decision.go 125 158 79.11%
internal/compatibility/proto/error.go 0 50 0.0%
internal/compatibility/thrift/response.go 134 258 51.94%
internal/compatibility/thrift/decision.go 0 148 0.0%
internal/compatibility/adapter.go 33 238 13.87%
internal/compatibility/thrift/enum.go 56 283 19.79%
internal/compatibility/proto/enum.go 56 295 18.98%
internal/compatibility/proto/response.go 0 260 0.0%
<!-- Total: 1289 5010 25.73% -->
Files with Coverage Reduction New Missed Lines %
internal/internal_task_pollers.go 2 80.45%
internal/compatibility/adapter.go 21 21.0%
<!-- Total: 23 -->
Totals Coverage Status
Change from base Build 824b8973-99ad-4f10-9ea7-7b04277109c0: -8.4%
Covered Lines: 12168
Relevant Lines: 19275

💛 - Coveralls