Skip to content

Conversation

@lostluck
Copy link
Contributor

@lostluck lostluck commented Feb 13, 2019

This change encapsulates internal implementation details that most users should not need.

In particular: This CL removes needing to wrap values in the exec.FullValue type before encoding them. It also hides the internal coder.Coder abstraction.

Previously users needed to do the following to use a beam coder to encode an element of a given type:

var t reflect.Type // from somewhere
coder := beam.NewCoder(typex.New(t))
enc := exec.MakeElementEncoder(beam.UnwrapCoder(coder))
var buf bytes.Buffer
err := enc.Encode(exec.FullValue{Elm: value}, &buf)
...

This required users to access beam implementation details directly: coder.Coder, the typex package, the exec package & the FullValue type.

Following this PR:

var t reflect.Type // from somewhere
enc := beam.NewElementEncoder(t)
var buf bytes.Buffer
err := enc.Encode(value, &buf)
...

which doesn't leak such details. Similarly for the Decoders.

This PR makes the breaking change of removing the beam.UnwrapCoder method. There is an open question of whether users will ever need to encode KVs, or CoGBKs or similar themselves, rather than via the framework, at which point we can revisit this.

TODO in a later PR: allow users to register coders that implement beam.ElementEncoder and beam.ElementDecoder.


Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Choose reviewer(s) and mention them in a comment (R: @username).
  • Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- --- --- --- ---
Java Build Status Build Status Build Status Build Status
Build Status
Build Status
Build Status Build Status Build Status
Python Build Status --- Build Status
Build Status
Build Status --- --- ---

@lostluck
Copy link
Contributor Author

R: @aaltay Review Please!

@aaltay aaltay merged commit b41c6b8 into apache:master Feb 13, 2019
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.

2 participants