robertwb commented on code in PR #26922:
URL: https://github.com/apache/beam/pull/26922#discussion_r1255040999
##########
sdks/python/apache_beam/transforms/core.py:
##########
@@ -2676,6 +2685,8 @@ def typed(transform):
def inject_default(_, combined):
if combined:
+ if len(combined) > 1:
+ _LOGGER.error('Cannot inject default values with: %s', combined)
Review Comment:
This error message isn't very actionable (or explanatory). What's going on
here is that if the PCollection was non-empty, the combined value is passed
through, and otherwise (the else clause below) a default value is provided.
Maybe "Multiple combined values unexpectedly provided for a global combine"
would be clearer?
##########
sdks/python/apache_beam/transforms/combine_globally_test.py:
##########
@@ -0,0 +1,55 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
Review Comment:
I have a slight preference for keeping things in the same file and being
consistent over having to look in multiple files, but won't object if you feel
strongly.
--
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]
