Skip to content

[BEAM-1928] Add Coder utilities for Proto conversions#2487

Closed
tgroh wants to merge 4 commits into
apache:masterfrom
tgroh:coders_utilities
Closed

[BEAM-1928] Add Coder utilities for Proto conversions#2487
tgroh wants to merge 4 commits into
apache:masterfrom
tgroh:coders_utilities

Conversation

@tgroh
Copy link
Copy Markdown
Member

@tgroh tgroh commented Apr 10, 2017

Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

  • Make sure the PR title is formatted like:
    [BEAM-<Jira issue #>] Description of pull request
  • Make sure tests pass via mvn clean verify. (Even better, enable
    Travis-CI on your fork and ensure the whole test matrix passes).
  • Replace <Jira issue #> in the title with the actual Jira issue
    number, if there is one.
  • If this contribution is large, please file an Apache
    Individual Contributor License Agreement.

@tgroh
Copy link
Copy Markdown
Member Author

tgroh commented Apr 10, 2017

R: @lukecwik

return toCustomCoder(coder);
}

private static RunnerApi.Coder toCustomCoder(Coder<?> coder) throws IOException {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it would be wise to just support component coders and known coder types upfront instead of migrating to it later.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 70.178% when pulling 9bd7dcf on tgroh:coders_utilities into 1c7a974 on apache:master.

@asfbot
Copy link
Copy Markdown

asfbot commented Apr 10, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/9380/
--none--

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.006%) to 70.182% when pulling 48ec9f2 on tgroh:coders_utilities into 1c7a974 on apache:master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.1%) to 70.037% when pulling 756f28f on tgroh:coders_utilities into 1c7a974 on apache:master.

@asfbot
Copy link
Copy Markdown

asfbot commented Apr 11, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/9387/
--none--

Copy link
Copy Markdown
Member

@lukecwik lukecwik left a comment

Choose a reason for hiding this comment

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

Minor changes and then LGTM

// TODO: standardize such things
public static final String CUSTOM_CODER_URN = "urn:beam:coders:javasdk:0.1";

// The URNs for coders are shared across languages
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

coders are shared --> coders which are shared

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done.

public class Coders {
private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();

// This URN says that the coder is just a UDF blob the indicated SDK understands
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

blob the indicated SDK --> blob this SDK

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done.

import org.apache.beam.sdk.util.WindowedValue;
import org.apache.beam.sdk.util.WindowedValue.FullWindowedValueCoder;

/** Utilities for working with {@link Coder Coders}. */
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

update comment:
Converts to and from Beam Runner API proto representations of {@link Coder Coders}.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done.

@asfgit asfgit closed this in 8beea73 Apr 11, 2017
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.02%) to 70.191% when pulling 07c27a6 on tgroh:coders_utilities into 1c7a974 on apache:master.

@asfbot
Copy link
Copy Markdown

asfbot commented Apr 11, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/9397/
--none--

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.

4 participants