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

google.cloud.bigquery.job.QueryJob.exception has wrong behavior #451

Closed
alexandreyc opened this issue Dec 24, 2020 · 8 comments · Fixed by #467
Closed

google.cloud.bigquery.job.QueryJob.exception has wrong behavior #451

alexandreyc opened this issue Dec 24, 2020 · 8 comments · Fixed by #467
Assignees
Labels
api: bigquery Issues related to the googleapis/python-bigquery 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

@alexandreyc
Copy link

Hello,

According to the documentation of google.cloud.bigquery.job.QueryJob.exception, this method should returns an exception (and not raises) if something went wrong. But according to my tests this is not always the case:

from google.cloud import bigquery

client = bigquery.Client()

# Correct behavior: returns an exception and does not raise.
query = 'WRONG QUERY'
job = client.query(query)
exc = job.exception()

# Wrong behavior: raises an exception instead of returning it.
query = 'ASSERT (SELECT 2 > 3) AS "Error !!!"'
job = client.query(query)
try:
    job.exception()
except:
    print('raised!')

Am I misunderstanding the documentation or is this the correct behavior?

I'm using google-cloud-bigquery==2.6.1 with Python 3.8.6. Let me know if you need more informations.

Thanks for your help,

@product-auto-label product-auto-label bot added the api: bigquery Issues related to the googleapis/python-bigquery API. label Dec 24, 2020
@yoshi-automation yoshi-automation added the triage me I really want to be triaged. label Dec 25, 2020
@HemangChothani HemangChothani added type: question Request for information or clarification. Not an issue. and removed triage me I really want to be triaged. labels Dec 28, 2020
@HemangChothani
Copy link
Contributor

HemangChothani commented Dec 28, 2020

@tswast @busunkim96 Please suggest.
I think something is missing for the scenario when query.state is RUNNING and library wants to retry it, it internally call the client._get_query_results method which throws an error of BadRequest.

I am not sure about to handle this in bigquery library and set the google.api_core.future.polling.PollingFuture.set_exception from the query.py file as error raised from google-cloud-core http client.

@alexandreyc
Copy link
Author

Working on something related, I discovered that the problem does not exist when doing a double assertion:

# Correct behavior: returns an exception and does not raise.
query = 'ASSERT (SELECT 2 > 3) AS "Error !!!"; ASSERT (SELECT 2 > 3) AS "Error again !!!";'
job = client.query(query)
exc = job.exception()

Hope it can help...

Thanks,

@HemangChothani
Copy link
Contributor

HemangChothani commented Dec 29, 2020

@tswast Backend consider "INVALID_ARGUMENT" and gave the reason "invalidQuery" when query contains ASSERT, but when query contains only one ASSERT like query = 'ASSERT (SELECT 2 > 3) AS "Error !!!"' it returns the 400 status code , hence exception raised from http client, but when query contains multiple ASSERT like 'ASSERT (SELECT 2 > 3) AS "Error !!!"; ASSERT (SELECT 2 > 3) AS "Error again !!!";', it returns 200 status code and the error result with message.

@tswast
Copy link
Contributor

tswast commented Jan 5, 2021

Regarding #451 (comment) I believe this is a difference in backend API behavior of getQueryResults between scripting jobs and non-scripting query jobs.

CC @shollyman

@tswast
Copy link
Contributor

tswast commented Jan 5, 2021

Regarding this issue, it appears that this library may be using the google-api-core library slightly incorrectly. The base class (where exception() is defined) assumes that _blocking_poll doesn't raise any exception

https://github.com/googleapis/python-api-core/blob/b51b7f587042fe9340371c1b5c8e9adf8001c43a/google/api_core/future/polling.py#L129-L134

Instead, it is expected that set_exception() is called.

We do this here:

self.set_exception(exception)

But we are missing the case when the getQueryResults backend API throws an error.

We probably need a try-catch block in QueryJob.done() as it is not expected (or documented) to raise an exception.

self._query_results = self._client._get_query_results(

If _get_query_results does raise an exception, we might have to inspect it a bit to determine if the job is complete (failed) or it was just a transient issue.

@tswast tswast 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. and removed type: question Request for information or clarification. Not an issue. labels Jan 5, 2021
@plamut
Copy link
Contributor

plamut commented Jan 7, 2021

@tswast Has this proposal been confirmed? If yes, I can start implementing it (as there's no assignee on the ticket yet).

@tswast
Copy link
Contributor

tswast commented Jan 7, 2021

Yes. Let's make sure that exception() (and also done()) does not throw.

@tswast
Copy link
Contributor

tswast commented Jan 7, 2021

result() should continue to throw (though it might throw because of set_exception instead of the current behavior that bubbles up from done())

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the googleapis/python-bigquery 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.

5 participants