Skip to content

Comments

[BEAM-1928] Add CombineTranslator#3199

Merged
asfgit merged 1 commit intoapache:masterfrom
tgroh:combine_translator
May 25, 2017
Merged

[BEAM-1928] Add CombineTranslator#3199
asfgit merged 1 commit intoapache:masterfrom
tgroh:combine_translator

Conversation

@tgroh
Copy link
Member

@tgroh tgroh commented May 22, 2017

This translates Combines to CombinePayloads and back

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.
  • 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
Member Author

tgroh commented May 22, 2017

R: @kennknowles

@tgroh tgroh force-pushed the combine_translator branch 2 times, most recently from 96b7878 to e898fdb Compare May 23, 2017 00:56
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.004%) to 70.664% when pulling e898fdb on tgroh:combine_translator into b633abe on apache:master.

public static CombinePayload toProto(
AppliedPTransform<?, ?, Combine.PerKey<?, ?, ?>> combine, SdkComponents sdkComponents)
throws IOException {
Coder<?> accumulatorCoder = null;
Copy link
Member

Choose a reason for hiding this comment

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

You'll actually need this coder. I added a hack and hit some other snags like unequal CountFn.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, there were a few things, they should have been fixed. PTAL

@tgroh tgroh force-pushed the combine_translator branch from e898fdb to 62fc723 Compare May 24, 2017 19:43
This translates Combines to CombinePayloads and back
@tgroh tgroh force-pushed the combine_translator branch from 62fc723 to 5b899a8 Compare May 24, 2017 20:05
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 70.696% when pulling 5b899a8 on tgroh:combine_translator into 6418bcf on apache:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 70.703% when pulling 5b899a8 on tgroh:combine_translator into 6418bcf on apache:master.

@tgroh
Copy link
Member Author

tgroh commented May 25, 2017

PTAL

Copy link
Member

@kennknowles kennknowles left a comment

Choose a reason for hiding this comment

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

LGTM

@asfgit asfgit merged commit 5b899a8 into apache:master May 25, 2017
asfgit pushed a commit that referenced this pull request May 25, 2017
jkff added a commit to jkff/incubator-beam that referenced this pull request May 25, 2017
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