You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@beam.apache.org by Robert Bradshaw <ro...@google.com.INVALID> on 2017/05/04 00:35:17 UTC

Re: Issue with Coder documentation regarding context

I filed a https://issues.apache.org/jira/browse/BEAM-2166 simply
removing these from the public API (for the reasons listed). We can
always bring them back in a forward compatible way if it turns out
that they're actually needed.

On Thu, Feb 9, 2017 at 1:18 PM, Jean-Baptiste Onofré <jb...@nanthrax.net> wrote:
> +1
>
> We should create a Jira and submit a PR about documentation.
>
> Regards
> JB
>
> On Feb 9, 2017, 14:12, at 14:12, Aviem Zur <av...@gmail.com> wrote:
>>Hi,
>>
>>I think improvements can be made to the documentation of `encode` and
>>`decode` methods in `Coder`.
>>
>>A coder may be used to encode/decode several objects using a single
>>stream,
>>you cannot assume that the stream the coder encodes to/decodes from
>>only
>>contains bytes representing a single object. For example, when the
>>coder is
>>used in an `IterableCoder`, for example in `GroupByKey`.
>>
>>When implementing a coder this needs to be taken into account.
>>
>>The `context` argument in `encode` and `decode` methods provides the
>>necessary information.
>>
>>The existing documentation for these methods does not seem to cover
>>this.
>>If users are not aware of this when implementing these methods it can
>>cause
>>errors or skewed results.
>>
>>See:
>>https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/coders/Coder.java#L126
>>and:
>>https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/coders/Coder.java#L137
>>
>>This is partially addressed in the documentation of the static
>>`Context`
>>values:
>>`OUTER`:
>>https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/coders/Coder.java#L72
>>and `NESTED`:
>>https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/coders/Coder.java#L79
>>
>>However, I think that the documentation of `encode` and `decode` should
>>explain this concept clearly, to avoid confusing users implementing
>>coders.