damccorm commented on code in PR #41159:
URL: https://github.com/apache/beam/pull/41159#discussion_r4531782887
##########
sdks/python/apache_beam/runners/common.py:
##########
@@ -1000,10 +1005,11 @@ def _invoke_process_per_window(
else:
window = GlobalWindow()
side_inputs = [si[window] for si in self.side_inputs]
- side_inputs.extend(additional_args)
+ if additional_args:
+ side_inputs.extend(additional_args)
args_for_process, kwargs_for_process = util.insert_values_in_args(
self.args_for_process, self.kwargs_for_process, side_inputs)
- if not self.recalculate_window_args:
+ if not self.recalculate_window_args and not additional_args:
Review Comment:
I made it a comment on the PR and not in the code because:
1) I think if you read through the code, this change makes sense (doesn't
need extra contextualization). If there are extra args added at runtime, then
we need to recalculate our full set of args.
2) In the context of this PR, though, it is surprising. This PR is mostly
non-functional perf-only changes, while this is an actual behavior change (just
a bug caught along the way)
If you disagree with (1), let me know and I'll add a code comment
--
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]
