Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
This repository was archived by the owner on Mar 6, 2026. It is now read-only.

test: make _AsyncJob tests mock at a lower layer#340

Merged
tswast merged 2 commits into
googleapis:mastergoogleapis/python-bigquery:masterfrom
tswast:issue339-asyncjobtswast/python-bigquery:issue339-asyncjobCopy head branch name to clipboard
Oct 26, 2020
Merged

test: make _AsyncJob tests mock at a lower layer#340
tswast merged 2 commits into
googleapis:mastergoogleapis/python-bigquery:masterfrom
tswast:issue339-asyncjobtswast/python-bigquery:issue339-asyncjobCopy head branch name to clipboard

Conversation

@tswast

@tswast tswast commented Oct 23, 2020

Copy link
Copy Markdown
Contributor

This is intented to make the _AsyncJob tests more robust
to changes in retry behavior. It also more explicitly tests
the retry behavior by observing API calls rather than calls
to certain methods.

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #339 🦕

This is intented to make the `_AsyncJob` tests more robust
to changes in retry behavior. It also more explicitly tests
the retry behavior by observing API calls rather than calls
to certain methods.
@tswast tswast requested a review from a team October 23, 2020 16:56
@google-cla google-cla Bot added the cla: yes This human has signed the Contributor License Agreement. label Oct 23, 2020
Comment thread tests/unit/test_job.py
# policy passed to it, so we don't throw a non-retriable
# exception here. See:
# https://github.com/googleapis/python-bigquery/issues/24
_make_retriable_exception(),

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@IlyaFaer You should be able to verify your changes in #41 by changing this from _make_retriable_exception() to NotFound as is done a few lines above.

raise NotImplementedError("Abstract")
return copy.deepcopy(self._properties)

_build_resource = to_api_repr # backward-compatibility alias

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all LGTM, but I'm curious what this backwards compatibility is for here. It's not clear to me why we were specifically testing to make sure this would raise NotImplementedError before.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear to me why we were specifically testing to make sure this would raise NotImplementedError before.

I think that was solely to make sure the coverage tests pass. We're still doing a bit of funny business in the *Job classes, in that most (all?) subclasses omit the statistics property from their to_api_repr.

I'm curious what this backwards compatibility is for here.

I don't remember, actually. I see a few hits for _build_resource in search. https://github.com/googleapis/python-bigquery/search?p=1&q=_build_resource I think _build_resource is usually the name we use for "update" methods that need to take a field mask and only populate certain fields. Jobs don't support update, so not sure why we ever had one.

@tswast tswast merged commit c9823d9 into googleapis:master Oct 26, 2020
@tswast tswast deleted the issue339-asyncjob branch October 26, 2020 15:04
gcf-merge-on-green Bot pushed a commit that referenced this pull request Oct 28, 2020
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.

Follow-up to #340
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

_AsyncJob test failure

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.