kennknowles commented on code in PR #37851:
URL: https://github.com/apache/beam/pull/37851#discussion_r2940927123
##########
sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/DoFn.java:
##########
@@ -284,6 +285,10 @@ public abstract <T> void outputWindowedValue(
Instant timestamp,
Collection<? extends BoundedWindow> windows,
PaneInfo paneInfo);
+
+ public abstract <T> void outputWindowedValue(TupleTag<T> tag,
WindowedValue<T> windowedValue);
+
+ public abstract void outputWindowedValue(WindowedValue<OutputT>
windowedValue);
Review Comment:
Ah, yes. The old interface between the SDK and runner v1 is that the runner
v1 provides a ProcessContext that will route the outputs to the correct place.
When we added OutputReceiver we just made it a wrapper instead of porting
runner v1 to the new design fully.
I think it is still OK to make the change you have here. The OutputBuilder
work was also to make sure that the metadata was propagated, so it is still
necessary. If a user choose to make a new WindowedValue from scratch and output
it... well, it is not ideal but it could be worse. TBH I wish to deprecate the
use of Contexts by users (ever since https://s.apache.org/a-new-dofn) since it
is a hopeless interface in some ways.
I think the only alternative is to adjust runner v1 (and other runners) to
implement OutputReceiver directly as though ProcessContext (and friends) do not
exist. Never invested in this because of runner v2 being the focus plus it
didn't seem too bad.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
