fix: keep google-auth transport imports out of Agent import - #7359
Closed
harshal-96 wants to merge 1 commit into
Closed
harshal-96 wants to merge 1 commit into
harshal-96 wants to merge 1 commit into
Conversation
_gcp_metadata imported google.auth.compute_engine._metadata and google.auth.transport.requests at module level. Both load requests, urllib3 and cryptography, so `from google.adk.agents import Agent` started paying for them and test_import_loading failed on main. Import them inside get_project_id_from_metadata, the only user.
Collaborator
|
thanks @harshal-96 for catching this and for sending the fix first. the same change landed in 41bebdc, which moves both google-auth imports into get_project_id_from_metadata, and it shipped in v2.11.0. please open a new issue if this is still happening. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
2. Or, if no issue exists, describe the change:
Problem:
test_import_loading.py::test_entry_point_loads_only_allowlisted_packages[agent]has failed onmainsince 8d8bbd2 (feat: default Vertex project from GCP metadata). No completed Continuous Integration run onmainhas passed since that commit: 11 failed and the others were cancelled. For example, https://github.com/google/adk-python/actions/runs/36484921667 fails this test on all five Python versions.The cause:
src/google/adk/utils/_gcp_metadata.pyimportsgoogle.auth.compute_engine._metadataandgoogle.auth.transport.requestsat module level.from google.adk.agents import Agentreaches that module throughmodels/google_llm.py, so every ADK process now loadsrequests,urllib3,charset_normalizerandcryptographyat startup.This is not in any release yet (2.10.0 has no
_gcp_metadata.py), so fixing it now keeps it out of the next one.Solution:
Both imports move into
get_project_id_from_metadata, the only function that uses them, inside its existingtry. If an import fails, the lookup returnsNonelike any other lookup failure.Each of the two imports on its own loads all four packages, so both have to move. The tests patch
pingandgeton the google-auth_metadatamodule itself, so they are unaffected.Testing Plan
Unit Tests:
No new test: the existing
test_import_loading.pycovers this, and it fails onmainand passes with this change.All results below are from environments built like CI (
uv sync --extra test --no-install-package lancedb):tests/unittests/test_import_loading.py,tests/unittests/utils,tests/unittests/models/test_google_llm.pyandtests/unittests/cli/utils/test_envs.py: 645 passed on Python 3.10, 3.11, 3.12, 3.13 and 3.14. Onmainthe same run gives 1 failed, 644 passed on each version.tests/unittestson Python 3.12: 16387 passed, 2 failed. The two failures are the LiveKitrun_livetests that also time out onmainon my machine, a separate bug in_merge_live_event_streams(Closing a live event stream early can hang forever in_merge_live_event_streams#7357).mainitself gives 16386 passed, 3 failed: those two plus the test fixed here.Manual End-to-End (E2E) Tests:
python -c "import sys; from google.adk.agents import Agent; print(sorted(m for m in ('requests', 'urllib3', 'cryptography', 'charset_normalizer') if m in sys.modules))"main:['charset_normalizer', 'cryptography', 'requests', 'urllib3'][]I have not run it on a Compute Engine VM against a real metadata server. The lookup code itself is unchanged; only where the two modules are imported moves.
Checklist