robertwb commented on a change in pull request #11835:
URL: https://github.com/apache/beam/pull/11835#discussion_r434730606



##########
File path: sdks/python/apache_beam/transforms/trigger_test.py
##########
@@ -518,6 +519,28 @@ def format_result(k_v):
                   'B-3': {10, 15, 16},
               }.items())))
 
+  def test_never(self):
+    with TestPipeline() as p:
+
+      def construct_timestamped(k_t):
+        return TimestampedValue((k_t[0], k_t[1]), k_t[1])
+
+      def format_result(k_v):
+        return ('%s-%s' % (k_v[0], len(k_v[1])), set(k_v[1]))
+
+      result = (
+          p
+          | beam.Create([1, 1, 2, 3, 4, 5, 10, 11])
+          | beam.FlatMap(lambda t: [('A', t), ('B', t + 5)])
+          | beam.Map(construct_timestamped)
+          | beam.WindowInto(
+              FixedWindows(10),
+              trigger=Never(),
+              accumulation_mode=AccumulationMode.DISCARDING)
+          | beam.GroupByKey()
+          | beam.Map(format_result))
+      assert_that(result, equal_to([]))

Review comment:
       I think even the concept of GC triggers (which have to be scheduled at a 
different time than a "normal" trigger at EOW) and late data is rather 
advanced. Also, yes, it used a custom (java serializable) WindowFn (didn't 
investigate which one this was). Possibly the right thing to do here would be 
to use "standard" triggers for the normal case, and advanced features when 
needed, assuming they can be queried at pipeline construction time. But this is 
completely an aside. 
   
   Thanks for the review. I discovered another couple of issues when running 
the full suite; I'll get them into separate PRs. 




----------------------------------------------------------------
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]


Reply via email to