Skip to content

Migrate []byte to proto Payload#324

Merged
alexshtin merged 21 commits into
temporalio:masterfrom
alexshtin:feature/metadata-blob
Apr 27, 2020
Merged

Migrate []byte to proto Payload#324
alexshtin merged 21 commits into
temporalio:masterfrom
alexshtin:feature/metadata-blob

Conversation

@alexshtin
Copy link
Copy Markdown
Contributor

What changed?
All user payload in proto files (mainly details, input, and result fields) was replaced from using []byte to commonpb.Payload.

Why?
commonpb.Payload has extra metadata attached to actual data. It closes #107.

How did you test it?
All automatic tests including go-sdk integration tests.

Potential risks
User data (such activity arguments and return types) will be serialized in a wrong way.

@alexshtin alexshtin requested review from a team, mfateev and samarabbas April 23, 2020 23:47
Comment thread common/codec/payload.go
dataConverter = encoded.GetDefaultDataConverter()
)

func EncodeString(str string) *commonpb.Payload {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is just some helpers because they are used in many places.

Copy link
Copy Markdown
Contributor

@shawnhathaway shawnhathaway Apr 24, 2020

Choose a reason for hiding this comment

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

Can we change package name to payloadcodec or change this to EncodeStringToPayload?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Moved to payload.

Comment thread common/codec/payload.go Outdated
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.

package codec
Copy link
Copy Markdown
Contributor

@shawnhathaway shawnhathaway Apr 24, 2020

Choose a reason for hiding this comment

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

Thoughts on serialization code moving to temporal-proto-go? Starting to notice a bit of duplication (see Cadence merge on sdk as well for examples of this)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, we need to do something around it. Let's discuss it one day.

@alexshtin alexshtin merged commit e76d2de into temporalio:master Apr 27, 2020
@alexshtin alexshtin deleted the feature/metadata-blob branch April 27, 2020 21:51
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.

Add metadata header to all blob payload fields

2 participants