kennknowles commented on a change in pull request #12067:
URL: https://github.com/apache/beam/pull/12067#discussion_r448646153
##########
File path: sdks/python/apache_beam/pipeline.py
##########
@@ -221,6 +221,14 @@ def __init__(self, runner=None, options=None, argv=None):
# then the transform will have to be cloned with a new label.
self.applied_labels = set() # type: Set[str]
+ # Create a context for assigning IDs to components. Ensures that any
+ # components that receive an ID during pipeline construction (for example
in
+ # ExternalTransform), will receive the same component ID when generating
the
+ # full pipeline proto.
+ from apache_beam.runners import pipeline_context
Review comment:
There's not actually a circular dependency of classes. You could move
this class in here and that would be fine.
##########
File path: sdks/python/apache_beam/runners/pipeline_context.py
##########
@@ -49,6 +50,32 @@
from apache_beam.coders.coder_impl import IterableStateWriter
+class ComponentIdContext(object):
Review comment:
There's a lot of overlap between the purpose of this and the purpose of
the `PipelineContext` but I can see how they are different. I see that the
pipeline only takes the context at `to_runner_api` time, and the context has
tweaks like `use_fake_coders` and `default_environment`. So when using xlang
expansion you can keep just the ids and throw away the contents that will be
generated according to these tweaks later.
##########
File path: sdks/python/apache_beam/transforms/external.py
##########
@@ -286,7 +286,8 @@ def expand(self, pvalueish):
pipeline = (
next(iter(self._inputs.values())).pipeline
if self._inputs else pvalueish.pipeline)
- context = pipeline_context.PipelineContext()
+ context = pipeline_context.PipelineContext(
+ component_id_context=pipeline._component_id_context)
Review comment:
Based on this use, maybe `_component_id_context` isn't private at all?
And maybe it is a `component_id_map` or some such. It isn't "context" in the
sense that it isn't "the stuff surrounding the thing you are interested in".
----------------------------------------------------------------
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
