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

'_AsyncQuery.cancel' fails to update from returned resource #2377

Closed
tseaver opened this issue Sep 21, 2016 · 5 comments
Closed

'_AsyncQuery.cancel' fails to update from returned resource #2377

tseaver opened this issue Sep 21, 2016 · 5 comments
Assignees
Labels
api: bigquery Issues related to the BigQuery API. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@tseaver
Copy link
Contributor

tseaver commented Sep 21, 2016

E.g.:

Traceback (most recent call last):
  File ...
    job.cancel()
  File ".../google/cloud/bigquery/job.py", line 378, in cancel
    self._set_properties(api_response)
  File ".../google/cloud/bigquery/job.py", line 262, in _set_properties
    self._scrub_local_properties(cleaned)
  File ".../google/cloud/bigquery/job.py", line 1050, in _scrub_local_properties
    configuration = cleaned['configuration']['query']
KeyError: 'configuration'

The docs for job.cancel show that the job resource is in a job subkey of the response.

@tseaver tseaver added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. api: bigquery Issues related to the BigQuery API. labels Sep 21, 2016
@tseaver tseaver self-assigned this Sep 21, 2016
@dhermes
Copy link
Contributor

dhermes commented Sep 21, 2016

@tseaver Germane to this change, I've run the following before to get a sense of our actual usage of streaming:

nosetests system_tests/storage.py system_tests/bigquery.py \
  --with-coverage  \
  --cover-branches  \
  --cover-tests  \
  --cover-package=gcloud.streaming  \
  --cover-package=gcloud.bigquery  \
  --cover-package=gcloud.storage  \
  --nocapture

Could be worthwhile to do it and see what code paths we aren't testing against the backend?

(NOTE: I'm aware this is using the old gcloud package and nosetests, sorry I am in a hurry and didn't update for the py.test equivalent.)

@tseaver
Copy link
Contributor Author

tseaver commented Sep 21, 2016

@dhermes I don't think that is germane to this issue, which doesn't mess with streaming at all. Maybe you meant #1431?

@dhermes
Copy link
Contributor

dhermes commented Sep 21, 2016

No I just meant that if you got a sense for lines uncovered during system tests then an error like this would be detectable. The streaming bit was just an example so you could do the same locally.

Essentially this error is due to the fact that our code and unit tests make a bad assumption about the payload.

@tseaver
Copy link
Contributor Author

tseaver commented Sep 21, 2016

OK. We don't have any system test which exercises _AsyncJob.cancel(): I found the problem while reproducing #2368.

I'm not convinced that we need to get to 100% coverage for system tests, but we should at least have "garden path" coverage for all the API requests.

@dhermes
Copy link
Contributor

dhermes commented Sep 21, 2016

Ha. Never heard "garden path" before! I agree having full coverage in system tests isn't necessary (or possible, since error cases may never occur), just wanted to share a script I had used before

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 BigQuery API. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

2 participants