Skip to content

[BEAM-5605] Add support for executing pair with restriction, split restriction, split and size restriction, process element and restriction and process sized element and restriction within the Java SDK harness.#10535

Merged
lukecwik merged 2 commits intoapache:masterfrom
lukecwik:splittabledofn2
Jan 10, 2020

Conversation

@lukecwik
Copy link
Member

@lukecwik lukecwik commented Jan 8, 2020

I removed the SplittableProcessElementsRunner and call the DoFnInvoker directly. I left the SplittableElementsInvoker as is since it is used by non portable SplittableDoFns.

I plan to have a follow-up change that removes the "context" object as it is no longer necessary.


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.

See the Contributor Guide for more tips on how to make review process smoother.

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- Build Status --- --- Build Status
Java Build Status Build Status 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
--- Build Status
Build Status
Build Status
Build Status
--- --- Build Status
XLang --- --- --- Build Status --- --- ---

Pre-Commit Tests Status (on master branch)

--- Java Python Go Website
Non-portable Build Status Build Status
Build Status
Build Status Build Status
Portable --- Build Status --- ---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

…striction, split and size restriction, process element and restriction and process sized element and restriction within the Java SDK harness.

I removed the SplittableProcessElementsRunner and call the DoFnInvoker directly.
I plan to have a follow-up change that removes the "context" object as it is no longer necessary.
@lukecwik
Copy link
Member Author

lukecwik commented Jan 8, 2020

R: @youngoli

Copy link
Contributor

@youngoli youngoli left a comment

Choose a reason for hiding this comment

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

Looks pretty good. Only caught one possible issue.

mainOutputConsumers,
(WindowedValue<OutputT>)
WindowedValue.of(
KV.of(currentElement.getValue(), output),
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this method also produce sizes, like the output method above?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, fixed in the version below.

I'm relying on the migration to using SDF everywhere will catch the edge cases via the validates runner tests since unit testing the execution of a single instance is quite verbose.

@lukecwik lukecwik merged commit 217afd5 into apache:master Jan 10, 2020
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