Skip to content

Conversation

@dannikay
Copy link
Contributor

@dannikay dannikay commented Dec 3, 2025

No description provided.

@github-actions github-actions bot added the python label Dec 3, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Dec 3, 2025

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

@dannikay
Copy link
Contributor Author

dannikay commented Dec 3, 2025

assign set of reviewers

@github-actions
Copy link
Contributor

github-actions bot commented Dec 3, 2025

Assigning reviewers:

R: @tvalentyn for label python.

Note: If you would like to opt out of this review, comment assign to next reviewer.

Available commands:

  • stop reviewer notifications - opt out of the automated review tooling
  • remind me after tests pass - tag the comment author after tests pass
  • waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

@tvalentyn
Copy link
Contributor

Thanks!

  1. @jrmccluskey would you recommedn we also add an integration test in sdks/python/apache_beam/ml/inference/gemini_inference_it_test.py ?

  2. There is a linter error

Running pylint...
************* Module apache_beam.ml.inference.gemini_inference_test
apache_beam/ml/inference/gemini_inference_test.py:20:0: W0611: Unused mock imported from unittest (unused-import)

@jrmccluskey
Copy link
Contributor

Seems like adding a small integration test case would be reasonable for this, just as an extra sanity check

@dannikay
Copy link
Contributor Author

dannikay commented Dec 4, 2025

Seems like adding a small integration test case would be reasonable for this, just as an extra sanity check

Integration testing isn't possible at the moment:

  • The API is not publicly available
  • Project apache-beam-testing is not yet allowlisted
  • We're not allowed to do automated testing against their non-public API (oneoff is fine but you need to jump through hoops to get allowlisted)

We can add one once the API is launched publicly. WDYT?

@dannikay
Copy link
Contributor Author

dannikay commented Dec 4, 2025

Thanks!

  1. @jrmccluskey would you recommedn we also add an integration test in sdks/python/apache_beam/ml/inference/gemini_inference_it_test.py ?
  2. There is a linter error

Running pylint... ************* Module apache_beam.ml.inference.gemini_inference_test apache_beam/ml/inference/gemini_inference_test.py:20:0: W0611: Unused mock imported from unittest (unused-import)

Removed the unused mock. Not sure why "tox -e py3-yapf" is not remove this line.

@tvalentyn
Copy link
Contributor

yapf changes formatting but doesn't modify content semantically.

@tvalentyn
Copy link
Contributor

actually mypy is still failing.

2025-12-04T21:05:36.1516076Z apache_beam/ml/inference/gemini_inference.py:193: error: Unexpected keyword argument "api_version" for "ClientOptions"  [call-arg]
2025-12-04T21:05:36.1517488Z target/.tox-mypy/mypy/lib/python3.10/site-packages/google/api_core/client_options.py:107: note: "ClientOptions" defined here
2025-12-04T21:05:36.1518654Z apache_beam/ml/inference/gemini_inference.py:193: error: Unexpected keyword argument "headers" for "ClientOptions"  [call-arg]
2025-12-04T21:05:36.1519875Z target/.tox-mypy/mypy/lib/python3.10/site-packages/google/api_core/client_options.py:107: note: "ClientOptions" defined here
2025-12-04T21:05:36.1520949Z apache_beam/ml/inference/gemini_inference.py:193: error: Unexpected keyword argument "timeout" for "ClientOptions"  [call-arg]
2025-12-04T21:05:36.1522892Z target/.tox-mypy/mypy/lib/python3.10/site-packages/google/api_core/client_options.py:107: note: "ClientOptions" defined here
2025-12-04T21:05:36.1524279Z apache_beam/ml/inference/gemini_inference.py:193: error: Argument "http_options" to "Client" has incompatible type "ClientOptions"; expected "HttpOptions | HttpOptionsDict | None"  [arg-type]

@dannikay
Copy link
Contributor Author

dannikay commented Dec 4, 2025

python -m pytest apache_beam/ml/inference/gemini_inference_test.py

Successfully set waiting utils config
=============================================================================================== test session starts ===============================================================================================
platform linux -- Python 3.13.5, pytest-8.3.4, pluggy-1.6.0
configfile: pytest.ini
plugins: timeout-2.4.0, xdist-3.8.0, requests-mock-1.12.1, hypothesis-6.148.3, anyio-4.12.0
timeout: 600.0s
timeout method: signal
timeout func_only: False
collected 8 items

apache_beam/ml/inference/gemini_inference_test.py ........ [100%]

================================================================================================ 8 passed in 3.32s ================================================================================================

@dannikay
Copy link
Contributor Author

dannikay commented Dec 4, 2025

actually mypy is still failing.

2025-12-04T21:05:36.1516076Z apache_beam/ml/inference/gemini_inference.py:193: error: Unexpected keyword argument "api_version" for "ClientOptions"  [call-arg]
2025-12-04T21:05:36.1517488Z target/.tox-mypy/mypy/lib/python3.10/site-packages/google/api_core/client_options.py:107: note: "ClientOptions" defined here
2025-12-04T21:05:36.1518654Z apache_beam/ml/inference/gemini_inference.py:193: error: Unexpected keyword argument "headers" for "ClientOptions"  [call-arg]
2025-12-04T21:05:36.1519875Z target/.tox-mypy/mypy/lib/python3.10/site-packages/google/api_core/client_options.py:107: note: "ClientOptions" defined here
2025-12-04T21:05:36.1520949Z apache_beam/ml/inference/gemini_inference.py:193: error: Unexpected keyword argument "timeout" for "ClientOptions"  [call-arg]
2025-12-04T21:05:36.1522892Z target/.tox-mypy/mypy/lib/python3.10/site-packages/google/api_core/client_options.py:107: note: "ClientOptions" defined here
2025-12-04T21:05:36.1524279Z apache_beam/ml/inference/gemini_inference.py:193: error: Argument "http_options" to "Client" has incompatible type "ClientOptions"; expected "HttpOptions | HttpOptionsDict | None"  [arg-type]

The ClientOption was swapped somehow by some autocompletion tool. I should have checked more thoroughly. Now verified locally that all unit tests pass.

@tvalentyn
Copy link
Contributor

there is still a linter error FYI.

@dannikay
Copy link
Contributor Author

dannikay commented Dec 4, 2025

there is still a linter error FYI.

it complains about this import syntax:

+++ /runner/_work/beam/beam/sdks/python/test-suites/tox/pycommon/build/srcs/sdks/python/apache_beam/ml/inference/gemini_inference.py:after	2025-12-04 21:47:38.526115
@@ -25,7 +25,8 @@
 
 from google import genai
 from google.genai import errors
-from google.genai.types import HttpOptions, Part
+from google.genai.types import HttpOptions
+from google.genai.types import Part
 from PIL.Image import Image

my local check passed:

yapf --diff apache_beam/ml/inference/gemini_inference.py
(base) xiaochu@xiaochu:~/github/beam/sdks/python$ pylint apache_beam/ml/inference/gemini_inference.py

--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

But anyway let me follow the check's preference.

@tvalentyn
Copy link
Contributor

yeah, isort is a separate check.

you might be able to also run gradlew lint locally.

@jrmccluskey do you by chance have instructions for running isort locally, similar to https://github.com/apache/beam/blob/master/contributor-docs/python-tips.md#running-yapf-formatter-manually ?

@dannikay
Copy link
Contributor Author

dannikay commented Dec 5, 2025

The remaining one failed check seems to be a flake:

Prepare all required actions
Getting action download info
Download action repository 'actions/setup-python@v5' (SHA:a26af69be951a213d495a4c3e4e4022e16d87065)
Download action repository 'actions/cache@v3' (SHA:6f8efc29b200d32929f49075959781ed54ec270c)
Download action repository 'actions/setup-java@v3' (SHA:17f84c3641ba7b8f6deff6309fc4c864478f5d62)
Download action repository 'gradle/gradle-build-action@v2' (SHA:a8f75513eafdebd8141bd1cd4e30fcd194af8dfa)
Download action repository 'actions/setup-go@v6' (SHA:4dc6199c7b1a012772edbd06daecab0f50c9053c)
Run ./.github/actions/setup-environment-action
Run actions/setup-python@v5
Installed versions
  Version 3.11 was not found in the local cache
  Version 3.11 is available for downloading
  Download from "https://github.com/actions/python-versions/releases/download/3.11.12-14343939122/python-3.11.12-linux-20.04-x64.tar.gz"
  Unexpected HTTP response: 404
  Error: Unexpected HTTP response: 404

@tvalentyn tvalentyn merged commit 72e84ef into apache:master Dec 5, 2025
100 of 101 checks passed
@apache apache deleted a comment from gemini-code-assist bot Dec 8, 2025
dannikay added a commit to dannikay/beam that referenced this pull request Dec 8, 2025
damccorm pushed a commit that referenced this pull request Dec 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants