robertwb commented on a change in pull request #12605:
URL: https://github.com/apache/beam/pull/12605#discussion_r473198225
##########
File path:
sdks/python/apache_beam/runners/portability/fn_api_runner/translations.py
##########
@@ -524,6 +524,22 @@ def add_parent(child, parent):
add_parent(parent, parents.get(parent))
components.transforms[parent].subtransforms.append(child)
+ copied_subtransform_ids = set()
Review comment:
Given that the set of subtransformation is a tree (no cycles) we can get
rid of this.
##########
File path:
sdks/python/apache_beam/runners/portability/fn_api_runner/translations.py
##########
@@ -541,6 +557,14 @@ def add_parent(child, parent):
components.transforms[transform_id].CopyFrom(transform)
add_parent(transform_id, stage.parent)
+ if partial:
Review comment:
I think we only want to copy sub-transforms of the transforms from line
552 above, perhaps move this logic up there? (Just a thought, maybe having the
method be called (`copy_subtransforms` and having it copy (recursively) just
the subtransforms could make this even easier to follow).
----------------------------------------------------------------
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]
