gemini-code-assist[bot] commented on code in PR #36342:
URL: https://github.com/apache/beam/pull/36342#discussion_r2395486948
##########
sdks/python/apache_beam/utils/transform_service_launcher.py:
##########
@@ -57,13 +57,13 @@ class TransformServiceLauncher(object):
# Maintaining a static list of launchers to prevent temporary resources
# from being created unnecessarily.
- def __new__(cls, project_name, port, beam_version=None):
+ def __new__(cls, project_name, port, beam_version=None, user_agent=None):
if project_name not in TransformServiceLauncher._launchers:
TransformServiceLauncher._launchers[project_name] = super(
TransformServiceLauncher, cls).__new__(cls)
return TransformServiceLauncher._launchers[project_name]
Review Comment:
The `__new__` method caches `TransformServiceLauncher` instances using only
`project_name` as the key. However, other parameters like `port` and
`beam_version` are crucial for the launcher's configuration. This can lead to
issues where a cached launcher for a given `project_name` is reused with a
different `port` or `beam_version`, causing `__init__` to be re-run with new
parameters on an old object. This can lead to incorrect behavior, such as
attempting to connect to a service on the wrong port. The cache key should
include all parameters that define a unique launcher configuration to prevent
such state corruption.
##########
sdks/python/apache_beam/transforms/external.py:
##########
@@ -1080,10 +1088,14 @@ def _default_args(self):
args.append('--alsoStartLoopbackWorker')
return args
+ def with_user_agent(self, user_agent: str):
+ self._user_agent = user_agent
+ return self
Review Comment:
The `with_user_agent` method modifies the instance in-place. This can lead
to unexpected side-effects if the `JavaJarExpansionService` instance is shared,
for example, across multiple pipelines with different `user_agent` options.
It's safer to treat `JavaJarExpansionService` instances as immutable and return
a new instance with the updated configuration.
```python
def with_user_agent(self, user_agent: str):
return JavaJarExpansionService(
self.path_to_jar,
self._extra_args,
self._classpath,
self._append_args,
user_agent=user_agent)
```
--
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]
