You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "Amar3tto (via GitHub)" <gi...@apache.org> on 2025/07/28 11:44:11 UTC

[PR] Fix PostCommit Python job [beam]

Amar3tto opened a new pull request, #35709:
URL: https://github.com/apache/beam/pull/35709

   Fixes #30513
   Successful tests: https://github.com/apache/beam/actions/runs/16547689442/job/46814158621
   
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [ ] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
    - [ ] Update `CHANGES.md` with noteworthy changes.
    - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://github.com/apache/beam/blob/master/CONTRIBUTING.md#make-the-reviewers-job-easier).
   
   To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
   
   GitHub Actions Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   [![Build python source distribution and wheels](https://github.com/apache/beam/actions/workflows/build_wheels.yml/badge.svg?event=schedule&&?branch=master)](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
   [![Python tests](https://github.com/apache/beam/actions/workflows/python_tests.yml/badge.svg?event=schedule&&?branch=master)](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Java tests](https://github.com/apache/beam/actions/workflows/java_tests.yml/badge.svg?event=schedule&&?branch=master)](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Go tests](https://github.com/apache/beam/actions/workflows/go_tests.yml/badge.svg?event=schedule&&?branch=master)](https://github.com/apache/beam/actions?query=workflow%3A%22Go+tests%22+branch%3Amaster+event%3Aschedule)
   
   See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI or the [workflows README](https://github.com/apache/beam/blob/master/.github/workflows/README.md) to see a list of phrases to trigger workflows.
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix PostCommit Python job [beam]

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm merged PR #35709:
URL: https://github.com/apache/beam/pull/35709


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix PostCommit Python job [beam]

Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on code in PR #35709:
URL: https://github.com/apache/beam/pull/35709#discussion_r2236045934


##########
sdks/python/apache_beam/examples/inference/gemini_text_classification.py:
##########
@@ -67,8 +67,18 @@ def parse_known_args(argv):
 
 class PostProcessor(beam.DoFn):
   def process(self, element: PredictionResult) -> Iterable[str]:
-    yield "Input: " + str(element.example) + " Output: " + str(
-        element.inference[1][0].content.parts[0].text)
+
+    inference = getattr(element, "inference", None)
+
+    if hasattr(inference[1], "content"):
+      yield inference[1].content.parts[0].text
+      return
+
+    if isinstance(inference[1], (tuple, list)) and len(inference) > 1:
+      yield "Input: " + str(element.example) + " Output: " + str(
+          inference[1][0].content.parts[0].text)
+    else:
+      yield "Can't decode inference for element: " + str(element.example)

Review Comment:
   ![critical](https://www.gstatic.com/codereviewagent/critical.svg)
   
   The current implementation is not robust against different shapes of the `inference` object and could lead to runtime errors.
   
   1.  `inference` is retrieved using `getattr(element, "inference", None)`. If `inference` is `None`, any subsequent access like `inference[1]` on lines 73 and 77 will raise a `TypeError`.
   2.  If `inference` is not a sequence or has fewer than two elements, `inference[1]` will raise a `TypeError` or `IndexError`.
   3.  On line 77, `isinstance(inference[1], (tuple, list))` is checked, but it's not verified if the list is non-empty before accessing `inference[1][0]`. This could cause an `IndexError`. The condition `len(inference) > 1` does not protect against this, as it checks the length of the parent `inference` object.
   
   To prevent these potential crashes, it's best to validate the structure of `inference` before using it. The suggested change introduces these checks for safer execution.
   
   ```python
       inference = getattr(element, "inference", None)
   
       if not isinstance(inference, (tuple, list)) or len(inference) < 2:
         yield "Can't decode inference for element: " + str(element.example)
         return
   
       prediction_payload = inference[1]
   
       if hasattr(prediction_payload, "content"):
         yield prediction_payload.content.parts[0].text
         return
   
       if isinstance(prediction_payload, (tuple, list)) and prediction_payload:
         yield "Input: " + str(element.example) + " Output: " + str(
             prediction_payload[0].content.parts[0].text)
       else:
         yield "Can't decode inference for element: " + str(element.example)
   ```



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix PostCommit Python job [beam]

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on code in PR #35709:
URL: https://github.com/apache/beam/pull/35709#discussion_r2237080122


##########
sdks/python/apache_beam/examples/inference/gemini_text_classification.py:
##########
@@ -67,8 +67,24 @@ def parse_known_args(argv):
 
 class PostProcessor(beam.DoFn):
   def process(self, element: PredictionResult) -> Iterable[str]:
-    yield "Input: " + str(element.example) + " Output: " + str(
-        element.inference[1][0].content.parts[0].text)
+
+    inference = getattr(element, "inference", None)
+
+    if not isinstance(inference, (tuple, list)) or len(inference) < 2:
+      yield "Can't decode inference for element: " + str(element.example)

Review Comment:
   Is the shape of the returned object actually different? Can we update the example to correctly extract the data here instead of just ignoring data?



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix PostCommit Python job [beam]

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on code in PR #35709:
URL: https://github.com/apache/beam/pull/35709#discussion_r2240212092


##########
sdks/python/apache_beam/examples/inference/gemini_text_classification.py:
##########
@@ -67,8 +67,24 @@ def parse_known_args(argv):
 
 class PostProcessor(beam.DoFn):
   def process(self, element: PredictionResult) -> Iterable[str]:
-    yield "Input: " + str(element.example) + " Output: " + str(
-        element.inference[1][0].content.parts[0].text)
+
+    inference = getattr(element, "inference", None)
+
+    if not isinstance(inference, (tuple, list)) or len(inference) < 2:
+      yield "Can't decode inference for element: " + str(element.example)

Review Comment:
   Lets either rework it in this PR or do something simpler here with a basic try/catch. Ideally, we'd just fix it here though, as it stands this isn't a great example



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix PostCommit Python job [beam]

Posted by "Amar3tto (via GitHub)" <gi...@apache.org>.
Amar3tto commented on code in PR #35709:
URL: https://github.com/apache/beam/pull/35709#discussion_r2237513205


##########
sdks/python/apache_beam/examples/inference/gemini_text_classification.py:
##########
@@ -67,8 +67,24 @@ def parse_known_args(argv):
 
 class PostProcessor(beam.DoFn):
   def process(self, element: PredictionResult) -> Iterable[str]:
-    yield "Input: " + str(element.example) + " Output: " + str(
-        element.inference[1][0].content.parts[0].text)
+
+    inference = getattr(element, "inference", None)
+
+    if not isinstance(inference, (tuple, list)) or len(inference) < 2:
+      yield "Can't decode inference for element: " + str(element.example)

Review Comment:
   It's a rare case, so I added these checks to prevent failing.
   Usually the original logic works



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix PostCommit Python job [beam]

Posted by "Amar3tto (via GitHub)" <gi...@apache.org>.
Amar3tto commented on code in PR #35709:
URL: https://github.com/apache/beam/pull/35709#discussion_r2237513205


##########
sdks/python/apache_beam/examples/inference/gemini_text_classification.py:
##########
@@ -67,8 +67,24 @@ def parse_known_args(argv):
 
 class PostProcessor(beam.DoFn):
   def process(self, element: PredictionResult) -> Iterable[str]:
-    yield "Input: " + str(element.example) + " Output: " + str(
-        element.inference[1][0].content.parts[0].text)
+
+    inference = getattr(element, "inference", None)
+
+    if not isinstance(inference, (tuple, list)) or len(inference) < 2:
+      yield "Can't decode inference for element: " + str(element.example)

Review Comment:
   I think it's a rare case, so I added these checks to prevent failing.
   Usually the original logic should work



##########
sdks/python/apache_beam/examples/inference/gemini_text_classification.py:
##########
@@ -67,8 +67,24 @@ def parse_known_args(argv):
 
 class PostProcessor(beam.DoFn):
   def process(self, element: PredictionResult) -> Iterable[str]:
-    yield "Input: " + str(element.example) + " Output: " + str(
-        element.inference[1][0].content.parts[0].text)
+
+    inference = getattr(element, "inference", None)
+
+    if not isinstance(inference, (tuple, list)) or len(inference) < 2:
+      yield "Can't decode inference for element: " + str(element.example)

Review Comment:
   Actually, it would be better to rework this, I was going to do it in another PR



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix PostCommit Python job [beam]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35709:
URL: https://github.com/apache/beam/pull/35709#issuecomment-3127955005

   Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment `assign set of reviewers`


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix PostCommit Python job [beam]

Posted by "Amar3tto (via GitHub)" <gi...@apache.org>.
Amar3tto commented on code in PR #35709:
URL: https://github.com/apache/beam/pull/35709#discussion_r2243042312


##########
sdks/python/apache_beam/examples/inference/gemini_text_classification.py:
##########
@@ -67,8 +67,24 @@ def parse_known_args(argv):
 
 class PostProcessor(beam.DoFn):
   def process(self, element: PredictionResult) -> Iterable[str]:
-    yield "Input: " + str(element.example) + " Output: " + str(
-        element.inference[1][0].content.parts[0].text)
+
+    inference = getattr(element, "inference", None)
+
+    if not isinstance(inference, (tuple, list)) or len(inference) < 2:
+      yield "Can't decode inference for element: " + str(element.example)

Review Comment:
   Done



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org