-
Notifications
You must be signed in to change notification settings - Fork 313
Bug introduced in v3.12.0 #1838
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
Comments
Thank you @jacek-jablonski for raising this issue! To ensure that I can reproduce the exact problem, could you provide a code snippet? This way I can add a unit test for this case. |
Hi @Linchin, here is a code snippet: import concurrent.futures
import pytest
from google.cloud import bigquery
TEST_SLEEP_QUERY = """
BEGIN
DECLARE DELAY_TIME DATETIME;
DECLARE WAIT STRING;
SET WAIT = 'TRUE';
SET DELAY_TIME = DATETIME_ADD(CURRENT_DATETIME, INTERVAL {seconds} SECOND);
WHILE WAIT = 'TRUE' DO
IF (DELAY_TIME < CURRENT_DATETIME) THEN
SET WAIT = 'FALSE';
END IF;
END WHILE;
END;
"""
def test_query_timeout():
client = bigquery.Client()
query_job = client.query(TEST_SLEEP_QUERY.format(seconds=10))
with pytest.raises(concurrent.futures.TimeoutError):
query_job.exception(timeout=1) |
Thank you @jacek-jablonski. I tried out the sample, and although the value of |
Hi @Linchin, for me, this test fails on
and passes on
|
I'm still unable to reproduce the issue - could you let me know which version of python you are using, and possibly the versions of your dependencies? |
I recorded a screencast: Nagranie.z.ekranu.2024-03-8.o.09.21.23.mp4Below some more information. Python version:
Pip freeze when v3.11.2 is installed:
|
Thanks @jacek-jablonski! I'm finally able to reproduce it in python 3.11. Let me take a deeper look into it. |
It's also flaky with python 3.11 - sometimes the exception is raised, sometimes it's not. Regardless I will fix the type check first. |
Correct the way we check whether `self._done_timeout` is an instance of `object` class or not. Fixes #1838 🦕
Hi,
I think a bug was introduced in v3.12.0, it this commit: 3645e32#diff-80c3339e876a6651c796df9b6621e6c8fba7d8b168f65239753398a0441b06a9R1345
isinstance(someinteger, object)
called like that is always True, becauseint
is an instance of anobject
:As a result, the timeout is always changed to
None
at this point. This causes timeout to not be respected when for exampleQueryJob.exception
method is called.The text was updated successfully, but these errors were encountered: