[ 
https://issues.apache.org/jira/browse/BEAM-2914?focusedWorklogId=558822&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-558822
 ]

ASF GitHub Bot logged work on BEAM-2914:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 26/Feb/21 23:21
            Start Date: 26/Feb/21 23:21
    Worklog Time Spent: 10m 
      Work Description: robertwb commented on a change in pull request #14095:
URL: https://github.com/apache/beam/pull/14095#discussion_r583978273



##########
File path: 
sdks/python/apache_beam/runners/portability/fn_api_runner/execution.py
##########
@@ -443,23 +661,22 @@ def _make_safe_windowing_strategy(self, id):
     windowing_strategy_proto = 
self.pipeline_components.windowing_strategies[id]
     if windowing_strategy_proto.window_fn.urn in SAFE_WINDOW_FNS:
       return id
-    elif (windowing_strategy_proto.merge_status ==
-          beam_runner_api_pb2.MergeStatus.NON_MERGING) or True:
+    else:
       safe_id = id + '_safe'
       while safe_id in self.pipeline_components.windowing_strategies:
         safe_id += '_'
       safe_proto = copy.copy(windowing_strategy_proto)
-      safe_proto.window_fn.urn = GenericNonMergingWindowFn.URN
-      safe_proto.window_fn.payload = (
-          windowing_strategy_proto.window_coder_id.encode('utf-8'))
+      if (windowing_strategy_proto.merge_status ==
+          beam_runner_api_pb2.MergeStatus.NON_MERGING):
+        safe_proto.window_fn.urn = GenericNonMergingWindowFn.URN
+        safe_proto.window_fn.payload = (
+            windowing_strategy_proto.window_coder_id.encode('utf-8'))
+      else:

Review comment:
       GenericMergingWindowFn handles arbitrary merging window fns over the 
FnAPI (I've been testing this with Java on the ULR.) Good point about the else 
though, explicitly testing for NEEDS_MERGE and throwing an exception otherwise.




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


Issue Time Tracking
-------------------

    Worklog Id:     (was: 558822)
    Time Spent: 10h 40m  (was: 10.5h)

> Fn API merging window support
> -----------------------------
>
>                 Key: BEAM-2914
>                 URL: https://issues.apache.org/jira/browse/BEAM-2914
>             Project: Beam
>          Issue Type: Improvement
>          Components: beam-model
>            Reporter: Henning Rohde
>            Priority: P3
>              Labels: portability
>          Time Spent: 10h 40m
>  Remaining Estimate: 0h
>
> The Fn API should support merging windows. Detailed design TBD.
> Once design is ready, expand subtasks similarly to BEAM-2822.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to