Fix proxy manipulation of single payload fields #202
Conversation
| @@ -273,6 +273,14 @@ func visitPayloads( | |||
| if options.SkipSearchAttributes { continue } | |||
| {{end}} | |||
| if o == nil { continue } | |||
| {{range $record.Payloads -}} | |||
There was a problem hiding this comment.
I wonder if we then should just remove the case *common.Payload logic altogether? Or if we want that to work for people passing in payload directly to the visitor, then we should probably just make case *common.Payload work (i.e. replace the data and metadata fields of the visitPayload result). And if that's the case, you don't need these other changes I assume.
So I say either 1) keep your logic and remove the case *common.Payload and document to users that this doesn't work when you pass in Payload directly, or 2) fix case *common.Payload and remove this new stuff.
There was a problem hiding this comment.
Ah yeah, we can remove the *common.Payload case. Let's just do that.
There was a problem hiding this comment.
Works for me. I would like to request that we update the VisitPayloads Godoc to mention that directly visiting *common.Payload does not work, but that is non-blocking (this is kinda a compat break too)
There was a problem hiding this comment.
Let me do this in a follow up PR since this is blocking a release and I won't have time to do it otherwise.
bergundy
force-pushed
the
proxy-single-payload-fix
branch
from
6926d41 to
be23dd9
Compare
## What was changed Updates SDK and Server dependencies. ## Why? Triaging a bug observed internally using a grpc-proxy with workflows that use a Nexus Operation. I wanted to reproduce with purely OSS sample code, which required updating dependencies to get the Nexus workflowservice methods. Note that there's a bug in the proxy that doesn't handle single Payload fields properly that will be fixed in temporalio/api-go#202. ## Checklist 1. How was this tested: Running both `grpc-proxy` and `nexus` samples. I didn't test the OIDC path, however. --------- Co-authored-by: Roey Berman <roey.berman@gmail.com>
| @@ -273,6 +273,14 @@ func visitPayloads( | |||
| if options.SkipSearchAttributes { continue } | |||
| {{end}} | |||
| if o == nil { continue } | |||
| {{range $record.Payloads -}} | |||
There was a problem hiding this comment.
Works for me. I would like to request that we update the VisitPayloads Godoc to mention that directly visiting *common.Payload does not work, but that is non-blocking (this is kinda a compat break too)
What changed?
Fixed a bug where the proxy visited single payload fields but didn't actually set the converted result on the parent object.