dmkozh commented on a change in pull request #13739:
URL: https://github.com/apache/beam/pull/13739#discussion_r564872946
##########
File path: sdks/python/apache_beam/runners/interactive/pipeline_instrument.py
##########
@@ -655,9 +660,13 @@ def process(
t=beam.DoFn.TimestampParam):
yield test_stream.WindowedValueHolder(WindowedValue(e, t, [w], p))
+ extended_target = pcoll
+ if self._is_streaming_pcollection(pcoll):
Review comment:
This is why I was interested in figuring out whether pcollection is not
streaming, so that windowing wouldn't matter. I don't fully understand the
code, but the comment to Reify suggests that wrapping is also needed outside
the visualization ('When it detects one, it puts the element into the correct
window then emits the value to downstream transforms').
It seems nice to have this just depend on the flag, but are we sure it would
work pcollections with non-trivial windowing? I also can imagine a case where
ib.collect is called with `include_window_info=False` at first and then with
`include_window_info=True`, which probably would result in an error.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
