Code cleanup now that all runners support windowed side inputs.#1812
Code cleanup now that all runners support windowed side inputs.#1812robertwb wants to merge 3 commits intoapache:python-sdkfrom
Conversation
|
Changes Unknown when pulling 6ff06f0 on robertwb:side-inputs into ** on apache:python-sdk**. |
|
Refer to this link for build results (access rights to CI server needed): |
| @@ -98,13 +97,9 @@ def __init__(self, | |||
| self.is_new_dofn = True | |||
|
|
|||
| # SideInputs | |||
There was a problem hiding this comment.
Can you add a docstring for the constructor of DoFnRunner? In particular, we should note that the expected type for the side_input argument is a sideinputs.SideInputMap.
| if isinstance(side_input, sideinputs.SideInputMap) | ||
| else {global_window: side_input} | ||
| for side_input in side_inputs] | ||
| self.side_inputs = side_inputs |
There was a problem hiding this comment.
This is a little confusing now that there is no processing code here; we only need to save the self.side_inputs property when we're using a NewDoFn; can you add a comment to this effect?
|
Thanks! Looks good modulo comments for clarity. |
|
Refer to this link for build results (access rights to CI server needed): |
|
Changes Unknown when pulling 0bbb51a on robertwb:side-inputs into ** on apache:python-sdk**. |
|
Refer to this link for build results (access rights to CI server needed): |
| fn: user DoFn to invoke | ||
| args: positional side input arguments (static and placeholder), if any | ||
| kwargs: keyword side input arguments (static and placeholder), if any | ||
| side_inputs: list of sideinput.SideInputMaps for deferred side inputs |
There was a problem hiding this comment.
nit: this should be "sideinputs.SideInputMaps"; period at end of each description.
|
Thanks! LGTM (modulo nit). |
4 participants
Be sure to do all of the following to help us incorporate your contribution
quickly and easily:
[BEAM-<Jira issue #>] Description of pull requestmvn clean verify. (Even better, enableTravis-CI on your fork and ensure the whole test matrix passes).
<Jira issue #>in the title with the actual Jira issuenumber, if there is one.
Individual Contributor License Agreement.