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

Remote codec-endpoint cherry-pick #420

Merged
merged 4 commits into from
Jan 29, 2024

Conversation

dandavison
Copy link
Contributor

@dandavison dandavison commented Jan 26, 2024

Cherry-pick of #384 with vendoring of sdk-go code.

@dandavison dandavison force-pushed the sdk-1194-codec-endpoint-cherry-pick branch from 2e95626 to f247329 Compare January 26, 2024 21:30
@dandavison dandavison marked this pull request as ready for review January 26, 2024 21:32
@dandavison dandavison force-pushed the sdk-1194-codec-endpoint-cherry-pick branch from f247329 to 4355588 Compare January 27, 2024 01:51
Copy link
Member

@cretz cretz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small note on some code that can be removed, but otherwise LGTM

client/codec.go Outdated
Comment on lines 116 to 204
type remoteDataConverter struct {
parent converter.DataConverter
payloadCodec converter.PayloadCodec
}

// NewRemoteDataConverter wraps the given parent DataConverter and performs
// encoding/decoding on the payload via the remote endpoint.
func NewRemoteDataConverter(parent converter.DataConverter, options converter.RemoteDataConverterOptions) converter.DataConverter {
options.Endpoint = strings.TrimSuffix(options.Endpoint, "/")
payloadCodec := NewRemotePayloadCodec(RemotePayloadCodecOptions(options))
return &remoteDataConverter{parent, payloadCodec}
}

// ToPayload implements DataConverter.ToPayload performing remote encoding on the
// result of the parent's ToPayload call.
func (rdc *remoteDataConverter) ToPayload(value interface{}) (*commonpb.Payload, error) {
payload, err := rdc.parent.ToPayload(value)
if payload == nil || err != nil {
return payload, err
}
encodedPayloads, err := rdc.payloadCodec.Encode([]*commonpb.Payload{payload})
if err != nil {
return payload, err
}
return encodedPayloads[0], err
}

// ToPayloads implements DataConverter.ToPayloads performing remote encoding on the
// result of the parent's ToPayloads call.
func (rdc *remoteDataConverter) ToPayloads(value ...interface{}) (*commonpb.Payloads, error) {
payloads, err := rdc.parent.ToPayloads(value...)
if payloads == nil || err != nil {
return payloads, err
}
encodedPayloads, err := rdc.payloadCodec.Encode(payloads.Payloads)
return &commonpb.Payloads{Payloads: encodedPayloads}, err
}

// FromPayload implements DataConverter.FromPayload performing remote decoding on the
// given payload before sending to the parent FromPayload.
func (rdc *remoteDataConverter) FromPayload(payload *commonpb.Payload, valuePtr interface{}) error {
decodedPayloads, err := rdc.payloadCodec.Decode([]*commonpb.Payload{payload})
if err != nil {
return err
}
return rdc.parent.FromPayload(decodedPayloads[0], valuePtr)
}

// FromPayloads implements DataConverter.FromPayloads performing remote decoding on the
// given payloads before sending to the parent FromPayloads.
func (rdc *remoteDataConverter) FromPayloads(payloads *commonpb.Payloads, valuePtrs ...interface{}) error {
if payloads == nil {
return rdc.parent.FromPayloads(payloads, valuePtrs...)
}

decodedPayloads, err := rdc.payloadCodec.Decode(payloads.Payloads)
if err != nil {
return err
}
return rdc.parent.FromPayloads(&commonpb.Payloads{Payloads: decodedPayloads}, valuePtrs...)
}

// ToString implements DataConverter.ToString performing remote decoding on the given
// payload before sending to the parent ToString.
func (rdc *remoteDataConverter) ToString(payload *commonpb.Payload) string {
if payload == nil {
return rdc.parent.ToString(payload)
}

decodedPayloads, err := rdc.payloadCodec.Decode([]*commonpb.Payload{payload})
if err != nil {
return err.Error()
}
return rdc.parent.ToString(decodedPayloads[0])
}

// ToStrings implements DataConverter.ToStrings using ToString for each value.
func (rdc *remoteDataConverter) ToStrings(payloads *commonpb.Payloads) []string {
if payloads == nil {
return nil
}

strs := make([]string, len(payloads.Payloads))
// Perform decoding one by one here so that we return individual errors
for i, payload := range payloads.Payloads {
strs[i] = rdc.ToString(payload)
}
return strs
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
type remoteDataConverter struct {
parent converter.DataConverter
payloadCodec converter.PayloadCodec
}
// NewRemoteDataConverter wraps the given parent DataConverter and performs
// encoding/decoding on the payload via the remote endpoint.
func NewRemoteDataConverter(parent converter.DataConverter, options converter.RemoteDataConverterOptions) converter.DataConverter {
options.Endpoint = strings.TrimSuffix(options.Endpoint, "/")
payloadCodec := NewRemotePayloadCodec(RemotePayloadCodecOptions(options))
return &remoteDataConverter{parent, payloadCodec}
}
// ToPayload implements DataConverter.ToPayload performing remote encoding on the
// result of the parent's ToPayload call.
func (rdc *remoteDataConverter) ToPayload(value interface{}) (*commonpb.Payload, error) {
payload, err := rdc.parent.ToPayload(value)
if payload == nil || err != nil {
return payload, err
}
encodedPayloads, err := rdc.payloadCodec.Encode([]*commonpb.Payload{payload})
if err != nil {
return payload, err
}
return encodedPayloads[0], err
}
// ToPayloads implements DataConverter.ToPayloads performing remote encoding on the
// result of the parent's ToPayloads call.
func (rdc *remoteDataConverter) ToPayloads(value ...interface{}) (*commonpb.Payloads, error) {
payloads, err := rdc.parent.ToPayloads(value...)
if payloads == nil || err != nil {
return payloads, err
}
encodedPayloads, err := rdc.payloadCodec.Encode(payloads.Payloads)
return &commonpb.Payloads{Payloads: encodedPayloads}, err
}
// FromPayload implements DataConverter.FromPayload performing remote decoding on the
// given payload before sending to the parent FromPayload.
func (rdc *remoteDataConverter) FromPayload(payload *commonpb.Payload, valuePtr interface{}) error {
decodedPayloads, err := rdc.payloadCodec.Decode([]*commonpb.Payload{payload})
if err != nil {
return err
}
return rdc.parent.FromPayload(decodedPayloads[0], valuePtr)
}
// FromPayloads implements DataConverter.FromPayloads performing remote decoding on the
// given payloads before sending to the parent FromPayloads.
func (rdc *remoteDataConverter) FromPayloads(payloads *commonpb.Payloads, valuePtrs ...interface{}) error {
if payloads == nil {
return rdc.parent.FromPayloads(payloads, valuePtrs...)
}
decodedPayloads, err := rdc.payloadCodec.Decode(payloads.Payloads)
if err != nil {
return err
}
return rdc.parent.FromPayloads(&commonpb.Payloads{Payloads: decodedPayloads}, valuePtrs...)
}
// ToString implements DataConverter.ToString performing remote decoding on the given
// payload before sending to the parent ToString.
func (rdc *remoteDataConverter) ToString(payload *commonpb.Payload) string {
if payload == nil {
return rdc.parent.ToString(payload)
}
decodedPayloads, err := rdc.payloadCodec.Decode([]*commonpb.Payload{payload})
if err != nil {
return err.Error()
}
return rdc.parent.ToString(decodedPayloads[0])
}
// ToStrings implements DataConverter.ToStrings using ToString for each value.
func (rdc *remoteDataConverter) ToStrings(payloads *commonpb.Payloads) []string {
if payloads == nil {
return nil
}
strs := make([]string, len(payloads.Payloads))
// Perform decoding one by one here so that we return individual errors
for i, payload := range payloads.Payloads {
strs[i] = rdc.ToString(payload)
}
return strs
}

I think you can remove all of this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that's right.

@dandavison dandavison merged commit 193953f into v0.10.8.rc Jan 29, 2024
4 of 5 checks passed
@dandavison dandavison deleted the sdk-1194-codec-endpoint-cherry-pick branch January 29, 2024 13:50
dandavison added a commit to dandavison/temporalio-cli that referenced this pull request Jan 29, 2024
Cherry-pick of temporalio#384 with vendoring
of sdk-go code.
dandavison added a commit that referenced this pull request Jan 29, 2024
This is #420 cherry-picked onto
`main`. We'll release either from `main` or `v0.10.8.rc` but either way
we want the chanes in `main`.

This replaces #384
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.

None yet

2 participants