gemini-code-assist[bot] commented on code in PR #35831:
URL: https://github.com/apache/beam/pull/35831#discussion_r2267005969
##########
sdks/python/apache_beam/testing/test_pipeline.py:
##########
@@ -96,7 +97,9 @@ def __init__(
included when construction the pipeline options object.
display_data (Dict[str, Any]): a dictionary of static data associated
with this pipeline that can be displayed when it runs.
-
+ timeout (int optional): The time in milliseconds to wait for pipeline to
finish before
+ raising timeout error. If not given, will wait indefinitely.
+
Review Comment:
The docstring for `timeout` has some formatting issues and could be more
precise. The type hint format is unconventional, the second line is not
correctly indented, and there's an extra blank line. Also, specifying that an
`AssertionError` is raised would be more accurate than 'timeout error'. A more
concise and conventional docstring would be better.
```suggestion
timeout (int, optional): Milliseconds to wait for the pipeline to
finish.
If the timeout is reached, an AssertionError is raised.
```
##########
sdks/python/apache_beam/testing/test_pipeline.py:
##########
@@ -116,7 +120,10 @@
test_runner_api=(
False if self.not_use_test_runner_api else test_runner_api))
if self.blocking:
- state = result.wait_until_finish()
+ if self.timeout is None:
+ state = result.wait_until_finish()
+ else:
+ state = result.wait_until_finish(duration=self.timeout)
Review Comment:
This `if/else` block can be simplified. The `wait_until_finish` method's
`duration` parameter defaults to `None`, so you can pass `self.timeout`
directly without the conditional check.
```python
state = result.wait_until_finish(duration=self.timeout)
```
--
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]
