[GitHub] [beam] robertwb commented on a change in pull request #11153: [BEAM-9537] Adding a new module for FnApiRunner
robertwb commented on a change in pull request #11153: [BEAM-9537] Adding a new module for FnApiRunner URL: https://github.com/apache/beam/pull/11153#discussion_r395897316 ## File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py ## @@ -269,8 +269,8 @@ def visit_transform(self, transform_node): pcoll.element_type, transform_node.full_label) key_type, value_type = pcoll.element_type.tuple_types if transform_node.outputs: -from apache_beam.runners.portability.fn_api_runner_transforms import \ - only_element +from apache_beam.runners.portability.fn_api_runner.transforms \ Review comment: I meant that utilities like only_element should probably be in a more common place rather than imported elsewhere from here. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [beam] robertwb commented on a change in pull request #11153: [BEAM-9537] Adding a new module for FnApiRunner
robertwb commented on a change in pull request #11153: [BEAM-9537] Adding a new module for FnApiRunner URL: https://github.com/apache/beam/pull/11153#discussion_r395885958 ## File path: sdks/python/apache_beam/runners/portability/fn_api_runner/fn_runner.py ## @@ -78,12 +78,12 @@ from apache_beam.runners import pipeline_context from apache_beam.runners import runner from apache_beam.runners.portability import artifact_service -from apache_beam.runners.portability import fn_api_runner_transforms from apache_beam.runners.portability import portable_metrics -from apache_beam.runners.portability.fn_api_runner_transforms import create_buffer_id -from apache_beam.runners.portability.fn_api_runner_transforms import only_element -from apache_beam.runners.portability.fn_api_runner_transforms import split_buffer_id -from apache_beam.runners.portability.fn_api_runner_transforms import unique_name +from apache_beam.runners.portability.fn_api_runner import transforms Review comment: Seeing this unqualified makes me realize that it's ambiguous with the notion of a PTransform (and the apache_beam.transforms package). Maybe we should call it `optimizations` or `translations`? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [beam] robertwb commented on a change in pull request #11153: [BEAM-9537] Adding a new module for FnApiRunner
robertwb commented on a change in pull request #11153: [BEAM-9537] Adding a new module for FnApiRunner URL: https://github.com/apache/beam/pull/11153#discussion_r395884481 ## File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py ## @@ -269,8 +269,8 @@ def visit_transform(self, transform_node): pcoll.element_type, transform_node.full_label) key_type, value_type = pcoll.element_type.tuple_types if transform_node.outputs: -from apache_beam.runners.portability.fn_api_runner_transforms import \ - only_element +from apache_beam.runners.portability.fn_api_runner.transforms \ Review comment: You don't have to do this in this PR, but this should probably be in apache_beam/utils. 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: us...@infra.apache.org With regards, Apache Git Services
