github-actions[bot] commented on code in PR #41200:
URL: https://github.com/apache/beam/pull/41200#discussion_r4535370168
##########
sdks/python/apache_beam/examples/inference/pytorch_image_captioning.py:
##########
@@ -622,12 +616,13 @@ def run(
try:
result.wait_until_finish(duration=1800000) # 30 min
finally:
- try:
- result.cancel()
- except Exception:
- logging.debug("Failed to cancel pipeline result.", exc_info=True)
-
if known_args.mode == 'streaming':
+ try:
+ result.cancel()
+ result.wait_until_finish(
Review Comment:
**🟡 Medium**: `DataflowPipelineResult.wait_until_finish()` raises
`DataflowRuntimeException` when the pipeline reaches a terminal state other
than `DONE` (such as `CANCELLED`). Although the `except Exception:` block
catches and suppresses this exception so `cleanup_pubsub_resources` still runs,
it logs a misleading failure message. Consider using the state-polling loop
introduced in `pytorch_imagenet_rightfit.py` (lines 525-533) for consistency.
##########
sdks/python/apache_beam/examples/inference/pytorch_image_object_detection.py:
##########
@@ -499,12 +499,13 @@ def run(
try:
result.wait_until_finish(duration=1800000) # 30 min
finally:
- try:
- result.cancel()
- except Exception:
- logging.debug("Failed to cancel pipeline result.", exc_info=True)
-
if known_args.mode == 'streaming':
+ try:
+ result.cancel()
+ result.wait_until_finish(
Review Comment:
**🟡 Medium**: Similar to `pytorch_image_captioning.py`, calling
`result.wait_until_finish()` after `result.cancel()` on Dataflow will raise
`DataflowRuntimeException` once the job enters `CANCELLED`. Consider using the
`PipelineState.CANCELLED` polling loop from `pytorch_imagenet_rightfit.py` here
as well.
##########
.github/workflows/beam_Inference_Python_Benchmarks_Dataflow.yml:
##########
@@ -249,9 +249,9 @@ jobs:
arguments: |
-PloadTest.mainClass=apache_beam.testing.benchmarks.inference.pytorch_image_object_detection_benchmarks
\
-Prunner=DataflowRunner \
- -PpythonVersion=3.11 \
+ -PpythonVersion=3.12 \
-PloadTest.requirementsTxtFile=apache_beam/ml/inference/pytorch_image_object_detection_requirements.txt
\
Review Comment:
**🟡 Medium**: These GPU steps now use
`--sdk_container_image=us.gcr.io/apache-beam-testing/python-postcommit-it/tensor_rt:latest`
with Python 3.12, but `pytorch_image_object_detection_requirements.txt` (lines
253, 297) and `pytorch_rightfit_requirements.txt` (lines 341, 352) still pin
`torchvision>=0.17.0,<0.21.0` alongside `torch>=2.2.0,<2.8.0`. If the
`tensor_rt:latest` image has `torch>=2.6.0` (which requires
`torchvision>=0.21.0`), pip will either downgrade `torch` or fail with a
`torch`/`torchvision` version mismatch on GPU workers unless those requirements
files are also updated to allow `torchvision<0.23.0`.
--
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]
