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

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

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



##########
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:
       Since there will be MergeStatus.ALREADY_MERGED, should we address that 
as well?
   I guess the implementation of  GenericMergingWindowFn handles custom window 
fn for python sdk only, should we throw exception here saying it is not 
supported yet when the ULR receives a window fn urn other than 
'beam:window_fn:pickled_python:v1'? I think this could happen in tests using 
java with ULR or xlang.




----------------------------------------------------------------
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: 558702)
    Time Spent: 10h 10m  (was: 10h)

> 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 10m
>  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