Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ValueError: staleness option can't be changed while a transaction is in progress #192

Closed
odino opened this issue Jan 17, 2022 · 17 comments · Fixed by #193
Closed

ValueError: staleness option can't be changed while a transaction is in progress #192

odino opened this issue Jan 17, 2022 · 17 comments · Fixed by #193
Assignees
Labels
api: spanner Issues related to the googleapis/python-spanner-sqlalchemy API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@odino
Copy link

odino commented Jan 17, 2022

Environment details

  • OS: Linux uname -a 5.11.0-46-generic #51~20.04.1-Ubuntu
  • Language runtime version: Python 3.8.10
  • Package versions:
google-cloud-spanner        3.12.1              
sqlalchemy-spanner          1.0.0 
grpc-google-iam-v1          0.12.3              
grpcio                      1.42.0              
grpcio-status               1.42.0              
grpcio-tools                1.38.0 
google-api-client           3.14.159265359.post1
google-api-core             2.3.0               
google-api-python-client    2.0.2               
google-auth                 1.28.0              
google-auth-httplib2        0.1.0               
google-cloud-core           2.2.1               
googleapis-common-protos    1.53.0              
grpc-google-iam-v1          0.12.3

Steps to reproduce

Running this script:

from sqlalchemy import create_engine

engine = create_engine(
    "spanner:///projects/X/instances/Y/databases/Z"
)

print(engine.execute("SELECT 1").scalar())

ends up running the query, but also throwing an error -- output from the CLI:

Exception during reset or similar
Traceback (most recent call last):
  File "/home/alex/.local/lib/python3.8/site-packages/sqlalchemy/pool/base.py", line 671, in _finalize_fairy
    fairy._reset(pool)
  File "/home/alex/.local/lib/python3.8/site-packages/sqlalchemy/pool/base.py", line 847, in _reset
    pool.dispatch.reset(self, self._connection_record)
  File "/home/alex/.local/lib/python3.8/site-packages/sqlalchemy/event/attr.py", line 259, in __call__
    fn(*args, **kw)
  File "/home/alex/.local/lib/python3.8/site-packages/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py", line 51, in reset_connection
    dbapi_conn.connection.staleness = None
  File "/home/alex/.local/lib/python3.8/site-packages/google/cloud/spanner_dbapi/connection.py", line 203, in staleness
    raise ValueError(
ValueError: `staleness` option can't be changed while a transaction is in progress. Commit or rollback the current transaction and try again.
1

Now, if I use:

with engine.begin() as connection:
    print(sql(connection, "SELECT 1 + 1").scalar())

it's all good. Any idea?

@odino odino added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Jan 17, 2022
@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/python-spanner-sqlalchemy API. label Jan 17, 2022
@IlyaFaer
Copy link
Contributor

IlyaFaer commented Jan 17, 2022

@odino, oh, I guess I know what's happening. By default, you're in SERIALIZABLE mode, which means you should commit your operations manually, or use a context manager, like you did:

with engine.begin() as connection:
    connection.execute(...)

Otherwise you're returning a connection back to connections pool, but there is a transaction in progress on the connection. As staleness drop is happening after operation execution, you see the results of the operation, but it fails later.

@odino
Copy link
Author

odino commented Jan 25, 2022

@IlyaFaer thanks for your pointer -- I do assume that is the problem. Though I see you're working on a PR, as it would be convenient not having to commit / roll it back manually eg. when you're executing just some random selects. I actually did not realize everything ran in the context of a tx here.

@odino
Copy link
Author

odino commented Jan 25, 2022

BTW one thing to note, when I switch to AUTOCOMMIT I still see some funky warning:

/home/alex/.local/lib/python3.8/site-packages/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py:947: UserWarning: This method is non-operational in autocommit mode
  dbapi_connection.rollback()

@vi3k6i5
Copy link
Contributor

vi3k6i5 commented Jan 28, 2022

The issue is resolved now, will do a release with the fix and update here.

@vi3k6i5
Copy link
Contributor

vi3k6i5 commented Jan 28, 2022

The fix is now released with the latest version.

@kazshinohara
Copy link

When v1.1.0 will be available at PyPI ?
https://pypi.org/project/sqlalchemy-spanner/#history

@larkee
Copy link
Contributor

larkee commented Feb 14, 2022

Hi @kazshinohara,

It looks like the release failed which is why it is not available on PyPI. Unfortunately, the issue for the failure was closed by a bot before we saw it so we weren't aware of this until now. I'm looking into this issue now and will let you know when it has been successfully published to PyPI.

Apologies for the inconvenience!

@kazshinohara
Copy link

@larkee Thanks !! I'm planning to talk about this ORM at an user community in Japan next month, hopefully the release wil be landed soon.

@meetchandan
Copy link

hey @vi3k6i5 -- as @kazshinohara mentioned, the v1.1.0 release has failed, any tentative date it woud be released?
We are facing this issue even after using engine.begin()

with engine.begin() as connection:
    sql(connection, "SELECT a from b").scalar()

@kazshinohara
Copy link

Hi any update on this ?

@ansh0l ansh0l assigned asthamohta and unassigned vi3k6i5 May 18, 2022
@ansh0l
Copy link
Contributor

ansh0l commented May 18, 2022

Hi @asthamohta , please take a look.

@ansh0l ansh0l reopened this May 18, 2022
@asthamohta
Copy link
Contributor

Hi @kazshinohara, @IlyaFaer has figured out the issue and will push a solution by tomorrow so hopefully by early next week we will have a release :)

@IlyaFaer
Copy link
Contributor

IlyaFaer commented Jun 6, 2022

I've checked the error on the current main branch - everything seems to be in order, no error messages:

Untitled

Fixed by #214

Closing the issue. In case the error remains after the next release, feel free to re-open.

@IlyaFaer IlyaFaer closed this as completed Jun 6, 2022
@asthamohta
Copy link
Contributor

asthamohta commented Jun 9, 2022

Hey @IlyaFaer, the customer is facing the following issues, can you look into them:

The first one is, it seems not to be able to pick up the sqlalchemy-spanner dialect unless I change the database URL from something like this:

"spanner:///projects/project-id/instances/instance-id/databases/database-id"
to this
"spanner+spanner:///projects/project-id/instances/instance-id/databases/database-id"

so, specifying the driver like that fixed the first issue.
and my current database URL setup look like this:

SQLALCHEMY_DATABASE_URI = f"mysql+pymysql://{PRIMA_DB_USERNAME}:{PRIMA_DB_PASSWORD}@{PRIMA_PROC_DB_HOST}/prima_processing"
SQLALCHEMY_BINDS = {
"jefe-jobs": "spanner+spanner:///projects/hpc-sandbox-268519/instances/jefe-spanner-instance/databases/jefe-jobs"
}

The second one is similar to the initial error (not the above one), but this time I get it every time I make a mysql related query it throws this error.

[2022-06-08 10:59:13,949] ERROR in base: Exception during reset or similar
Traceback (most recent call last):
File "/pdata/devsrc/amhadest/wavepipe-service/venv/lib/python3.8/site-packages/sqlalchemy/pool/base.py", line 697, in _finalize_fairy
fairy._reset(pool)
File "/pdata/devsrc/amhadest/wavepipe-service/venv/lib/python3.8/site-packages/sqlalchemy/pool/base.py", line 874, in _reset
pool.dispatch.reset(self, self._connection_record)
File "/pdata/devsrc/amhadest/wavepipe-service/venv/lib/python3.8/site-packages/sqlalchemy/event/attr.py", line 261, in call
fn(*args, **kw)
File "/pdata/devsrc/amhadest/wavepipe-service/venv/lib/python3.8/site-packages/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py", line 51, in reset_connection
if dbapi_conn.connection.inside_transaction:
AttributeError: 'Connection' object has no attribute 'inside_transaction'

@asthamohta asthamohta reopened this Jun 9, 2022
@IlyaFaer
Copy link
Contributor

The second one is similar to the initial error (not the above one), but this time I get it every time I make a mysql related query it throws this error

But, wait, when you create a connection to a Spanner database, SQLAlchemy uses our dialect for all the operations. It's not possible to use this dialect with MySQL, because Spanner and MySQL are different. If I understand your case correctly, you should use two different connections for two different databases - one for Spanner, another one for MySQL. By looking at the database URL prefixes, SQLAlchemy will understand what kind of a connection and dialect should be used. But it must be two separate connections/engines/dialects.

I can add a condition into the reset mechanism not to do anything with no-Spanner connections, but I don't think it'll allow to run MySQL queries on Spanner dialect.

@amha19
Copy link

amha19 commented Jun 20, 2022

Apologies for the late response. @IlyaFaer that make sense. It seems the changes made in this fork and this change both can fix the issue I was having even though I’m using a single engine for both Spanner and MySQL (Thank you for that).
Although the error is gone, if possible, I’m trying to create two different engines and connections, one for Spanner and one for MySQL. The problem is I’m using Flask-SQLAlchemy, and as far as I understand Flask-SQLAlchemy accepts only one SQLALCHEMY_DATABASE_URI and if one need to connect to multiple database, it has to be done through the SQLALCHEMY_BINDS method (as it shown in the code snippet above).
I understand as it is mentioned in this issue (#221 ), even creating separate engines throws the same error but I thought it would be preferable to have separate engines and connections. So, Is there a way to create separate connections using Flask-sqlalchemy so that they can use different engine and their own dialect?

@IlyaFaer
Copy link
Contributor

@amha19, if you'll do something like this:

engine = create_engine( "spanner+spanner:///...")
engine2 = create_engine("mysql+mysql:///...") 

Engines and connections will be separate (and it's what Flask is doing, as far as I can tell). But all the connections will be using the same reset mechanism. That was the problem of the original issue - connections reset mechanism is global for all the engines and connections on your machine - that's something we didn't expect.

Thus, if I understand correctly what you mean, it's already working like this. Connections are separate, it's the connections reset mechanism which brings confusion because of its globality (but from now on this mechanism first checks what kind of a connection it got before doing the actual reset).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/python-spanner-sqlalchemy API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants