Skip to content
This repository was archived by the owner on Feb 23, 2026. It is now read-only.

feat(api-core): pass retry from result() to done() #9

Merged
gcf-merge-on-green merged 12 commits into from
Oct 16, 2020
Merged

feat(api-core): pass retry from result() to done() #9

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
512a65f
feat(api-core): pass retry from result() to done()
Feb 21, 2020
a5288b2
fix tests
Feb 21, 2020
b6a0655
Merge branch 'master' into retry_into_done
busunkim96 Mar 3, 2020
3c58c28
Merge branch 'master' into retry_into_done
Oct 7, 2020
e8c1de4
pass retry into done() only if it is not default
Oct 7, 2020
0f145ac
Merge branch 'master' into retry_into_done
tswast Oct 12, 2020
812a2e5
add more unit tests
Oct 13, 2020
f6b1749
Merge branch 'retry_into_done' of https://github.com/q-logic/python-a…
Oct 13, 2020
6e411ec
use super() in the overriden result() method
Oct 13, 2020
0c17181
fix super() function call for py-2.x
Oct 13, 2020
2b21471
add kwargs guard
Oct 15, 2020
57468f8
Merge branch 'master' into retry_into_done
tswast Oct 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions google/api_core/future/polling.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,16 +78,16 @@ def done(self, retry=DEFAULT_RETRY):
# pylint: disable=redundant-returns-doc, missing-raises-doc
raise NotImplementedError()

def _done_or_raise(self):
def _done_or_raise(self, retry=DEFAULT_RETRY):
"""Check if the future is done and raise if it's not."""
if not self.done():
if not self.done(retry):
Copy link
Copy Markdown
Contributor

@tswast tswast Oct 6, 2020

Choose a reason for hiding this comment

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

This should be passed as a keyword argument

Suggested change
if not self.done(retry):
if not self.done(retry=retry):

Copy link
Copy Markdown
Contributor

@tswast tswast Oct 6, 2020

Choose a reason for hiding this comment

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

On further thought, since this is a new kwarg, we don't want to force all clients that subclass it to add it in order to use the next google-api-core (this is a breaking change).

We should really only populate retry at all when explicitly passed a value for one.

Maybe something like this?

Suggested change
if not self.done(retry):
done_kwargs = {}
if retry is not DEFAULT_RETRY:
done_kwargs["retry"] = retry
if not self.done(**done_kwargs):

raise _OperationNotComplete()

def running(self):
"""True if the operation is currently running."""
return not self.done()

def _blocking_poll(self, timeout=None):
def _blocking_poll(self, timeout=None, retry=DEFAULT_RETRY):
"""Poll and wait for the Future to be resolved.

Args:
Expand All @@ -101,13 +101,13 @@ def _blocking_poll(self, timeout=None):
retry_ = self._retry.with_deadline(timeout)

try:
retry_(self._done_or_raise)()
retry_(self._done_or_raise)(retry)
Comment thread
IlyaFaer marked this conversation as resolved.
Outdated
except exceptions.RetryError:
raise concurrent.futures.TimeoutError(
"Operation did not complete within the designated " "timeout."
)

def result(self, timeout=None):
def result(self, timeout=None, retry=DEFAULT_RETRY):
"""Get the result of the operation, blocking if necessary.

Args:
Expand All @@ -122,7 +122,7 @@ def result(self, timeout=None):
google.api_core.GoogleAPICallError: If the operation errors or if
the timeout is reached before the operation completes.
"""
self._blocking_poll(timeout=timeout)
self._blocking_poll(timeout=timeout, retry=retry)
Comment thread
IlyaFaer marked this conversation as resolved.
Outdated

if self._exception is not None:
# pylint: disable=raising-bad-type
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/future/test_polling.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def __init__(self):
self.poll_count = 0
self.event = threading.Event()

def done(self):
def done(self, retry=polling.DEFAULT_RETRY):
self.poll_count += 1
self.event.wait()
self.set_result(42)
Expand All @@ -108,7 +108,7 @@ def test_result_with_polling():


class PollingFutureImplTimeout(PollingFutureImplWithPoll):
Comment thread
IlyaFaer marked this conversation as resolved.
def done(self):
def done(self, retry=polling.DEFAULT_RETRY):
time.sleep(1)
return False

Expand All @@ -130,7 +130,7 @@ def __init__(self, errors):
super(PollingFutureImplTransient, self).__init__()
self._errors = errors

def done(self):
def done(self, retry=polling.DEFAULT_RETRY):
if self._errors:
error, self._errors = self._errors[0], self._errors[1:]
raise error("testing")
Expand Down