feat(sqlalchemy-bigquery): add mypy session and resolve type errors#17030
feat(sqlalchemy-bigquery): add mypy session and resolve type errors#17030chalmerlowe wants to merge 6 commits into
Conversation
Removes the skip for SQLAlchemy >= 2.0 on test_compiled_query_literal_binds. Modifies the test to wrap the compiled query in sqlalchemy.text(str(compiled)) before execution, as passing a Compiled object directly to Connection.execute() is not supported in SQLAlchemy 2.0. Fixes sqlalchemy-bigquery portion of Issue #16042.
There was a problem hiding this comment.
Code Review
This pull request introduces static type checking for the package by adding a mypy configuration and updating the noxfile to run mypy across all supported Python versions. The changes include adding type hints, assertions, and 'type: ignore' pragmas to satisfy the type checker, as well as updating several method signatures to maintain compatibility with modern SQLAlchemy. A significant regression was identified in the client creation logic where a string was incorrectly assigned to a variable expecting a ClientInfo object, which would lead to an AttributeError at runtime.
| project_id = default_project | ||
|
|
||
| client_info = google_client_info(user_agent=user_agent) | ||
| client_info = user_agent if user_agent is not None else google_client_info() |
There was a problem hiding this comment.
The logic for assigning client_info appears to be incorrect and introduces a regression. google_client_info (typically an alias for google.api_core.gapic_v1.client_info.ClientInfo) is a class that should be instantiated to return a ClientInfo object. The original code google_client_info(user_agent=user_agent) correctly handled both None and string values for user_agent to produce a valid ClientInfo instance. The new code assigns the raw user_agent string to client_info when it is not None, which will cause an AttributeError when bigquery.Client attempts to access ClientInfo methods (such as to_user_agent()) on what is now a string object.
| client_info = user_agent if user_agent is not None else google_client_info() | |
| client_info = google_client_info(user_agent=user_agent) |
This PR is a partial fix for: #16014
Summary of changes:
mypysession tonoxfile.py.mypy.iniwith appropriate settings and ignores for optional dependencies (geoalchemy2,shapely,alembic,pybigquery) that do not have type hinting OR that are evaluated for the purposes of warning users._helpers.py,_struct.py,_types.py, andbase.pyby using more specific type handling, assertions, and adding explicit comments for remainingtype: ignorepragmas.test_catalog_functions.pyrelated toNullTypeinstantiation.