zhoufek commented on a change in pull request #15603:
URL: https://github.com/apache/beam/pull/15603#discussion_r727124005
##########
File path: sdks/python/apache_beam/transforms/trigger_test.py
##########
@@ -470,93 +470,58 @@ def
test_after_watermark_no_allowed_lateness_safe_late(self):
0,
DataLossReason.NO_POTENTIAL_LOSS)
- def test_after_watermark_safe_late(self):
+ def test_after_watermark_allowed_lateness_safe_late(self):
self._test(
AfterWatermark(late=DefaultTrigger()),
60,
DataLossReason.NO_POTENTIAL_LOSS)
- def test_after_watermark_no_allowed_lateness_may_finish_late(self):
- self._test(
- AfterWatermark(late=AfterProcessingTime()),
- 0,
- DataLossReason.NO_POTENTIAL_LOSS)
-
- def test_after_watermark_may_finish_late(self):
- self._test(
- AfterWatermark(late=AfterProcessingTime()),
- 60,
- DataLossReason.NO_POTENTIAL_LOSS)
-
- def test_after_watermark_no_allowed_lateness_condition_late(self):
- self._test(
- AfterWatermark(late=AfterCount(5)), 0,
DataLossReason.NO_POTENTIAL_LOSS)
-
- def test_after_watermark_condition_late(self):
- self._test(
- AfterWatermark(late=AfterCount(5)),
- 60,
- DataLossReason.NO_POTENTIAL_LOSS)
-
- def test_after_count_one(self):
- self._test(AfterCount(1), 0, DataLossReason.MAY_FINISH)
-
- def test_after_count_gt_one(self):
- self._test(
- AfterCount(2),
- 0,
- DataLossReason.MAY_FINISH | DataLossReason.CONDITION_NOT_GUARANTEED)
+ def test_after_count(self):
+ self._test(AfterCount(42), 0, DataLossReason.MAY_FINISH)
def test_repeatedly_safe_underlying(self):
self._test(
Repeatedly(DefaultTrigger()), 0, DataLossReason.NO_POTENTIAL_LOSS)
- def test_repeatedly_may_finish_underlying(self):
- self._test(Repeatedly(AfterCount(1)), 0, DataLossReason.NO_POTENTIAL_LOSS)
-
- def test_repeatedly_condition_underlying(self):
- self._test(Repeatedly(AfterCount(2)), 0, DataLossReason.NO_POTENTIAL_LOSS)
+ def test_repeatedly_unsafe_underlying(self):
+ self._test(Repeatedly(AfterCount(42)), 0, DataLossReason.NO_POTENTIAL_LOSS)
- def test_after_any_some_unsafe(self):
+ def test_after_any_one_may_finish(self):
self._test(
- AfterAny(AfterCount(1), DefaultTrigger()),
- 0,
- DataLossReason.NO_POTENTIAL_LOSS)
-
- def test_after_any_same_reason(self):
- self._test(
- AfterAny(AfterCount(1), AfterProcessingTime()),
+ AfterAny(AfterCount(42), DefaultTrigger()),
0,
DataLossReason.MAY_FINISH)
- def test_after_any_different_reasons(self):
+ def test_after_any_all_safe(self):
self._test(
- AfterAny(AfterCount(2), AfterProcessingTime()),
+ AfterAny(Repeatedly(AfterCount(42)), DefaultTrigger()),
0,
- DataLossReason.MAY_FINISH | DataLossReason.CONDITION_NOT_GUARANTEED)
-
- def test_after_all_some_unsafe(self):
- self._test(
- AfterAll(AfterCount(1), DefaultTrigger()), 0,
DataLossReason.MAY_FINISH)
+ DataLossReason.NO_POTENTIAL_LOSS)
- def test_after_all_safe(self):
+ def test_after_all_some_may_finish(self):
self._test(
- AfterAll(Repeatedly(AfterCount(1)), DefaultTrigger()),
+ AfterAll(AfterCount(1), DefaultTrigger()),
Review comment:
I don't think it will? `_ParallelTriggerFn.on_fire` takes a "combine_op"
of all the sub-triggers' `TriggerFn.on_fire` results. For `AfterAll`, that's
Python's `all` function. Since `AfterCount` always returns True[1] and
`DefaultTrigger` always returns False[2], this would be equivalent to
`all([True, False])`, which always returns False, meaning this instance of
`AfterAll` will never finish.
[1]
https://github.com/apache/beam/blob/ef4364519c94e4bff83bb0f32aace95c3093ad16/sdks/python/apache_beam/transforms/trigger.py#L670-L671
[2]
https://github.com/apache/beam/blob/ef4364519c94e4bff83bb0f32aace95c3093ad16/sdks/python/apache_beam/transforms/trigger.py#L335-L336
--
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]
