aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBu Sun Kim <8822365+busunkim96@users.noreply.github.com>2021-10-26 11:40:20 -0600
committerGitHub <noreply@github.com>2021-10-26 17:40:20 +0000
commit9e6091ee59a30e72a6278b369f6a08e7aef32f22 (patch)
tree9e16aa4d6087807470b8ead46fb702fa16adc470
parent240edb6a822482865a10ff0bcd5439ccf38d73a3 (diff)
downloadpython-api-core-9e6091ee59a30e72a6278b369f6a08e7aef32f22.tar.gz
fix: revert "fix: do not error on LROs with no response or error" (#294)
Reverts googleapis/python-api-core#258 From @truchle > A while ago you helped me submit this pull request for the Python LRO client library. This was about making the LRO library not throw an error when receiving an empty LRO response. It turns out that the Python LRO library has always been behaving correctly by accepting empty LRO responses. It would just throw an error if the response field is NULL (see screenshot). > > Since then I've consulted other LRO client library owners (@vam-google ) and it turns out that the fault was in the CCAI Insights server code. CCAI Insights was returning NULL responses instead of empty responses (more context here b/202432905). Since then the issue has been fixed in our server and has rolled out to production, so reverting the Python LRO false fix wouldn't break our code sample.
-rw-r--r--google/api_core/operation.py10
-rw-r--r--google/api_core/operation_async.py10
-rw-r--r--tests/asyncio/test_operation_async.py6
-rw-r--r--tests/unit/test_operation.py6
4 files changed, 17 insertions, 15 deletions
diff --git a/google/api_core/operation.py b/google/api_core/operation.py
index a66e4a5..b17f753 100644
--- a/google/api_core/operation.py
+++ b/google/api_core/operation.py
@@ -140,11 +140,11 @@ class Operation(polling.PollingFuture):
)
self.set_exception(exception)
else:
- # 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())
+ exception = exceptions.GoogleAPICallError(
+ "Unexpected state: Long-running operation had neither "
+ "response nor error set."
+ )
+ self.set_exception(exception)
def _refresh_and_update(self, retry=polling.DEFAULT_RETRY):
"""Refresh the operation and update the result if needed.
diff --git a/google/api_core/operation_async.py b/google/api_core/operation_async.py
index 17624d6..6bae865 100644
--- a/google/api_core/operation_async.py
+++ b/google/api_core/operation_async.py
@@ -136,11 +136,11 @@ class AsyncOperation(async_future.AsyncFuture):
)
self.set_exception(exception)
else:
- # 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())
+ exception = exceptions.GoogleAPICallError(
+ "Unexpected state: Long-running operation had neither "
+ "response nor error set."
+ )
+ self.set_exception(exception)
async def _refresh_and_update(self, retry=async_future.DEFAULT_RETRY):
"""Refresh the operation and update the result if needed.
diff --git a/tests/asyncio/test_operation_async.py b/tests/asyncio/test_operation_async.py
index 886b1c8..26ad7ce 100644
--- a/tests/asyncio/test_operation_async.py
+++ b/tests/asyncio/test_operation_async.py
@@ -158,7 +158,7 @@ async def test_exception():
@mock.patch("asyncio.sleep", autospec=True)
@pytest.mark.asyncio
-async def test_done_with_no_error_or_response(unused_sleep):
+async def test_unexpected_result(unused_sleep):
responses = [
make_operation_proto(),
# Second operation response is done, but has not error or response.
@@ -166,9 +166,9 @@ async def test_done_with_no_error_or_response(unused_sleep):
]
future, _, _ = make_operation_future(responses)
- result = await future.result()
+ exception = await future.exception()
- assert isinstance(result, struct_pb2.Struct)
+ assert "Unexpected state" in "{!r}".format(exception)
def test_from_gapic():
diff --git a/tests/unit/test_operation.py b/tests/unit/test_operation.py
index bafff8b..22e23bc 100644
--- a/tests/unit/test_operation.py
+++ b/tests/unit/test_operation.py
@@ -169,7 +169,7 @@ def test_exception_with_error_code():
assert isinstance(exception, exceptions.NotFound)
-def test_done_with_no_error_or_response():
+def test_unexpected_result():
responses = [
make_operation_proto(),
# Second operation response is done, but has not error or response.
@@ -177,7 +177,9 @@ def test_done_with_no_error_or_response():
]
future, _, _ = make_operation_future(responses)
- assert isinstance(future.result(), struct_pb2.Struct)
+ exception = future.exception()
+
+ assert "Unexpected state" in "{!r}".format(exception)
def test__refresh_http():