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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: do not error on LROs with no response or error #258

Merged
merged 6 commits into from Aug 31, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions google/api_core/operation.py
Expand Up @@ -140,11 +140,11 @@ def _set_result_from_operation(self):
)
self.set_exception(exception)
else:
exception = exceptions.GoogleAPICallError(
"Unexpected state: Long-running operation had neither "
"response nor error set."
)
self.set_exception(exception)
# Some APIs set `done: true`, with an empty response.
# Set the result to an empty message of the expected
# result type.
# https://google.aip.dev/151
self.set_result(self._result_type())
Copy link
Contributor

Choose a reason for hiding this comment

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

Coverage thinks we need a final else: here (for the case that self._operation.done is False).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

馃憤 I switched this back to else: as self._operation.done must already be True to get to this clause.


def _refresh_and_update(self, retry=polling.DEFAULT_RETRY):
"""Refresh the operation and update the result if needed.
Expand Down
10 changes: 5 additions & 5 deletions google/api_core/operation_async.py
Expand Up @@ -136,11 +136,11 @@ def _set_result_from_operation(self):
)
self.set_exception(exception)
else:
exception = exceptions.GoogleAPICallError(
"Unexpected state: Long-running operation had neither "
"response nor error set."
)
self.set_exception(exception)
# Some APIs set `done: true`, with an empty response.
# Set the result to an empty message of the expected
# result type.
# https://google.aip.dev/151
self.set_result(self._result_type())
Copy link
Contributor

Choose a reason for hiding this comment

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

Coverage thinks we need a final else: here (for the case that self._operation.done is False).


async def _refresh_and_update(self, retry=async_future.DEFAULT_RETRY):
"""Refresh the operation and update the result if needed.
Expand Down
6 changes: 3 additions & 3 deletions tests/asyncio/test_operation_async.py
Expand Up @@ -153,17 +153,17 @@ async def test_exception():

@mock.patch("asyncio.sleep", autospec=True)
@pytest.mark.asyncio
async def test_unexpected_result(unused_sleep):
async def test_done_with_no_error_or_response(unused_sleep):
responses = [
make_operation_proto(),
# Second operation response is done, but has not error or response.
make_operation_proto(done=True),
]
future, _, _ = make_operation_future(responses)

exception = await future.exception()
result = await future.result()

assert "Unexpected state" in "{!r}".format(exception)
assert isinstance(result, struct_pb2.Struct)


def test_from_gapic():
Expand Down
6 changes: 2 additions & 4 deletions tests/unit/test_operation.py
Expand Up @@ -163,17 +163,15 @@ def test_exception_with_error_code():
assert isinstance(exception, exceptions.NotFound)


def test_unexpected_result():
def test_done_with_no_error_or_response():
responses = [
make_operation_proto(),
# Second operation response is done, but has not error or response.
make_operation_proto(done=True),
]
future, _, _ = make_operation_future(responses)

exception = future.exception()

assert "Unexpected state" in "{!r}".format(exception)
assert isinstance(future.result(), struct_pb2.Struct)


def test__refresh_http():
Expand Down