bug: handle SQLAlchemy connection deprecation #717
Conversation
product-auto-label
bot
added
the
api: spanner
rafi-rr
force-pushed
the
driver_connection
branch
from
4734349 to
dbbe2ab
Compare
| from google.api_core.client_options import ClientOptions | ||
| from google.auth.credentials import AnonymousCredentials | ||
| from google.cloud.spanner_v1 import Client, TransactionOptions | ||
| from sqlalchemy.exc import NoSuchTableError |
There was a problem hiding this comment.
nit: can you remove the unrelated changes from this pull request (and potentially open a separate pull request for formatting issues, if any)
| if dbapi_conn.inside_transaction: | ||
| transaction_started = getattr( | ||
| dbapi_conn, | ||
| "spanner_transaction_started", |
There was a problem hiding this comment.
Note: The name of the property is _spanner_transaction_started.
But I don't think it is wise to make this change at all, or at least not in the way that it is done here. _spanner_transaction_started is a private property, meaning that it could silently be removed or changed at any time. It would be better to continue to rely only on the inside_transaction property, even though it is deprecated, as it is part of the public API.
|
Closing this PR as it was intended as a suggestion. I believe the team will make more informed decisions on how to handle the deprecation issue. |
2 participants
Fixes #715
connectiontodriver_connection