chore: Enforce unused-import linting with ruff and clean up src#6092
Closed
Jacksunwei wants to merge 5 commits into
Closed
chore: Enforce unused-import linting with ruff and clean up src#6092Jacksunwei wants to merge 5 commits into
Jacksunwei wants to merge 5 commits into
Conversation
Add a ruff F401 (unused-import) check, wired into pyproject.toml and a ruff pre-commit hook scoped to src/, so dead imports are caught automatically instead of relying on IDE highlights. Remove 117 genuinely unused imports across src/. Imports that look unused but are intentional are preserved: - Re-export hubs use the `import X as X` convention (e.g. dependencies shims, adk_web_server, a2a executor utils, tools/api_registry). - Optional-dependency availability probes inside try/except and the test-coupled / defensive imports use `# noqa: F401`. __init__.py files are exempted from F401 since their imports are intentional public re-exports. Also update the bigquery data_insights stale test to patch _gda_stream_util.get_stream (the actual code path) instead of the removed requests import. Change-Id: Ib1f8ed07af3e1969584ae5cc827c745be239edec
The scripts/run_precommit_checks.py runner (used in no-git checkouts where pre-commit can't run) treats any config hook id missing from _HOOK_SPECS as a failure. Adding the ruff hook to the config without this would break the script. Map `ruff` to `ruff check` / `ruff check --fix`, pin ruff in the dev extra to match .pre-commit-config.yaml, and strip a duplicate `--fix` from the config args so check mode stays read-only and fix mode does not pass `--fix` twice. Change-Id: Ic6467c6e14fc96a96aac9cf2e8e3a0e6aaea4e7d
Fix a pre-existing mypy [no-redef] on Context._output_for_ancestors (annotate once before the if/else instead of in both branches); it only surfaced because this PR edits context.py. Exclude four files that hardcode googleapis.com URLs from ruff (bigquery_credentials, data_insights_tool, bigquery_agent_analytics_plugin, data_agent_tool) and revert their import removals so they leave the PR diff. The check-file-contents mTLS policy check fires on any changed file containing a googleapis.com string -- here mostly OAuth scope URLs with no .mtls variant -- so cleaning their imports would force unrelated, and in some cases incorrect, mTLS changes. The standalone runner passes --force-exclude so the exclude is honored when files are listed explicitly (the ruff pre-commit hook already does this by default). Also revert the related bigquery data_insights test. Change-Id: If7b8de2da29635d3815bd4ca517e4742e58df623
These two files carry pre-existing formatting drift on main and entered this PR's diff range via rebase, failing the pre-commit CI check. Reformatting them (line-wrapping only, no logic change) clears the check. Change-Id: I031b17f2d7923d6b4abae9468c04030c96715f9a
Apply isort to agent_identity/gcp_auth_provider.py and add the missing trailing newline to adk-unit-guide/SKILL.md. Both carry pre-existing drift on main that entered this PR's pre-commit scope via rebase; neither is a logic change. Change-Id: I05574279ef99fffdd20b30042b4c12b5400dd6ab
Collaborator
Author
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
1 participant
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.
Summary
pyproject.tomland a ruff pre-commit hook scoped tosrc/, so dead imports are caught automatically instead of relying on IDE highlights.src/.import X as Xconvention (dependency shimsdependencies/vertexai.py&dependencies/rouge_scorer.py,cli/adk_web_server.py,a2a/executor/utils.py,tools/api_registry.py).try/except(e.g.import vertexai,import sqlalchemy,import multipart) and a couple of test-coupled / defensive imports use# noqa: F401.__init__.pyfiles are exempted from F401 (their imports are intentional public re-exports).data_insightsstale test to patch_gda_stream_util.get_stream(the actual code path) instead of the removedrequestsimport.tests/cleanup is intentionally out of scope for this PR and will follow separately; the hook is scoped tosrc/so it stays green.Test plan
uvx ruff check src/→ All checks passeduv run pytest tests/unittests→ 7206 passed (pre-existinggoogle-genaiversion-conflict telemetry test and missing-google.antigravityerrors excluded; both fail on clean main)import google.adksmoke test passes