-
Notifications
You must be signed in to change notification settings - Fork 306
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
feat: add reload
argument to *Job.done()
functions
#341
feat: add reload
argument to *Job.done()
functions
#341
Conversation
This enables checking the job status without making an API call. It also fixes an inconsistency in `QueryJob`, where a job can be reported as "done" without having the results of a `getQueryResults` API call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do the changes here indicate there's further changes for registering the done callback? My mental model has them independent, but I wanted to make sure.
is_done = ( | ||
# Only consider a QueryJob complete when we know we have the final | ||
# query results available. | ||
self._query_results is not None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checking: What about DDL statements, where we have no results? Or would this already retain the empty row iterator?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DDL statements don't 404 for jobs.getQueryResults
, do they? This is the full response object of jobs.getQueryResults
, not just the rows.
No callbacks here. Maybe when we implement the async client #18
|
Maybe you meant -- How do we get the needed arguments to |
This enables checking the job status without making an API call.
It also fixes an inconsistency in
QueryJob
, where a job can bereported as "done" without having the results of a
getQueryResults
APIcall.
Follow-up to #340