Fix DoFnTester side inputs #659
Conversation
The side inputs were being stored as iterables, but being returned as the raw type. Store the side input values directly instead.
|
R: @kennknowles |
| return WindowedValue.valueInGlobalWindow(input); | ||
| } | ||
| })); | ||
| public <T> void setSideInput( |
There was a problem hiding this comment.
Why this change? The additional method for just a single value seems fine, but the case for adding globally windowed data remains. At the least, update the javadoc to indicate that you aren't setting the entirety of the PCollection contents. Is it unused?
There was a problem hiding this comment.
I've done a bit of cleanup, including removing the setSideInput without window method.
The other two commits are mainly cleanup of dead (or effectively dead) code.
tgroh
force-pushed
the
do_fn_tester_side_inputs
branch
from
fa4a3ae
to
fbb0265
Compare
Side Outputs are appended to the map of outputs on-demand.
No in-package representation refers to the fields of DoFnTester directly.
tgroh
force-pushed
the
do_fn_tester_side_inputs
branch
from
fbb0265
to
29adfdb
Compare
|
LGTM. Nice catch. I'll merge in a couple minutes, let Travis have a chance to catch up. |
| */ | ||
| public void setSideOutputTags(TupleTagList sideOutputTags) { |
There was a problem hiding this comment.
This line is mostly a diff failure.
I believe that both Side Input behaviors are reasonable; although I can also see an argument for throwing an illegal state exception if the (PCollectionView, BoundedWindow) was not specified, and adding a setSideInputDefault(PCollectionView, BoundedWindow) method.
Be sure to do all of the following to help us incorporate your contribution
quickly and easily:
[BEAM-<Jira issue #>] Description of pull requestmvn clean verify. (Even better, enableTravis-CI on your fork and ensure the whole test matrix passes).
<Jira issue #>in the title with the actual Jira issuenumber, if there is one.
Individual Contributor License Agreement.
The side inputs were being stored as iterables, but being returned as
the raw type.
Store the side input values directly instead.