From 0e3f9bd922046c9a96162ea8ea8a65adce32a61a Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Fri, 19 May 2023 16:23:15 +0000 Subject: [PATCH 1/6] fix: add actionable errors for GCE long running operations --- google/api_core/exceptions.py | 13 +++++++++++-- google/api_core/extended_operation.py | 4 ++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/google/api_core/exceptions.py b/google/api_core/exceptions.py index 35f2a6f8..012768a8 100644 --- a/google/api_core/exceptions.py +++ b/google/api_core/exceptions.py @@ -142,10 +142,19 @@ def __init__(self, message, errors=(), details=(), response=None, error_info=Non self._error_info = error_info def __str__(self): + error_msg = "{} {}".format(self.code, self.message) if self.details: - return "{} {} {}".format(self.code, self.message, self.details) + error_msg = "{} {}".format(error_msg, self.details) else: - return "{} {}".format(self.code, self.message) + if self.errors: + errors = [ + f"{error.code}: {error.message}" + for error in self.errors + if hasattr(error, "code") and hasattr(error, "message") + ] + if len(errors) > 0: + error_msg = "".join(error_msg, "\n".join(errors)) + return error_msg @property def reason(self): diff --git a/google/api_core/extended_operation.py b/google/api_core/extended_operation.py index 79d47f0d..ca050868 100644 --- a/google/api_core/extended_operation.py +++ b/google/api_core/extended_operation.py @@ -158,10 +158,14 @@ def _handle_refreshed_operation(self): return if self.error_code and self.error_message: + errors = [] + if hasattr(self, "error") and hasattr(self.error, "errors"): + errors = self.error.errors exception = exceptions.from_http_status( status_code=self.error_code, message=self.error_message, response=self._extended_operation, + errors=errors, ) self.set_exception(exception) elif self.error_code or self.error_message: From 0703fc87750d5ced454e93388ecb39ebf1ca0999 Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Thu, 1 Jun 2023 12:54:16 +0000 Subject: [PATCH 2/6] add unit test --- google/api_core/exceptions.py | 3 +-- tests/unit/test_extended_operation.py | 23 ++++++++++++++++++++++- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/google/api_core/exceptions.py b/google/api_core/exceptions.py index 012768a8..ae8f6968 100644 --- a/google/api_core/exceptions.py +++ b/google/api_core/exceptions.py @@ -152,8 +152,7 @@ def __str__(self): for error in self.errors if hasattr(error, "code") and hasattr(error, "message") ] - if len(errors) > 0: - error_msg = "".join(error_msg, "\n".join(errors)) + error_msg = "{} {}".format(error_msg, "\n".join(errors)) return error_msg @property diff --git a/tests/unit/test_extended_operation.py b/tests/unit/test_extended_operation.py index c551bfa8..e7ce7f6f 100644 --- a/tests/unit/test_extended_operation.py +++ b/tests/unit/test_extended_operation.py @@ -33,11 +33,21 @@ class StatusCode(enum.Enum): DONE = 1 PENDING = 2 + class LROCustomErrors: + class LROCustomError: + def __init__(self, code: str = None, message: str = None): + self.code = code + self.message = message + + def __init__(self, errors: typing.List[LROCustomError] = None): + self.errors = errors + name: str status: StatusCode error_code: typing.Optional[int] = None error_message: typing.Optional[str] = None armor_class: typing.Optional[int] = None + error: typing.Optional[LROCustomErrors] = None # Note: in generated clients, this property must be generated for each # extended operation message type. @@ -155,19 +165,30 @@ def test_done_w_retry(): def test_error(): + _EXCEPTION_CODE = "INCOMPATIBLE_BACKEND_SERVICES" + _EXCEPTION_MESSAGE = "Validation failed for instance group" responses = [ CustomOperation( name=TEST_OPERATION_NAME, status=CustomOperation.StatusCode.DONE, error_code=400, error_message="Bad request", + error=CustomOperation.LROCustomErrors( + errors=[ + CustomOperation.LROCustomErrors.LROCustomError( + code=_EXCEPTION_CODE, message=_EXCEPTION_MESSAGE + ) + ] + ), ), ] ex_op, _, _ = make_extended_operation(responses) # Defaults to CallError when grpc is not installed - with pytest.raises(exceptions.BadRequest): + with pytest.raises( + exceptions.BadRequest, match=f"{_EXCEPTION_CODE}: {_EXCEPTION_MESSAGE}" + ): ex_op.result() # Inconsistent result From 0e5ce8966c3d4759be413c499426117590a6b8a1 Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Thu, 1 Jun 2023 12:58:29 +0000 Subject: [PATCH 3/6] mypy --- tests/unit/test_extended_operation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_extended_operation.py b/tests/unit/test_extended_operation.py index e7ce7f6f..a850b1a6 100644 --- a/tests/unit/test_extended_operation.py +++ b/tests/unit/test_extended_operation.py @@ -35,11 +35,11 @@ class StatusCode(enum.Enum): class LROCustomErrors: class LROCustomError: - def __init__(self, code: str = None, message: str = None): + def __init__(self, code: str = "", message: str = ""): self.code = code self.message = message - def __init__(self, errors: typing.List[LROCustomError] = None): + def __init__(self, errors: typing.List[LROCustomError] = []): self.errors = errors name: str From 46dfd9742601fdb45086468add4e0f36f7d406c3 Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Thu, 1 Jun 2023 13:04:18 +0000 Subject: [PATCH 4/6] add notes that the workaround should be removed once proposal A from b/284179390 is implemented --- google/api_core/exceptions.py | 2 ++ google/api_core/extended_operation.py | 2 ++ tests/unit/test_extended_operation.py | 2 ++ 3 files changed, 6 insertions(+) diff --git a/google/api_core/exceptions.py b/google/api_core/exceptions.py index ae8f6968..87157d44 100644 --- a/google/api_core/exceptions.py +++ b/google/api_core/exceptions.py @@ -145,6 +145,8 @@ def __str__(self): error_msg = "{} {}".format(self.code, self.message) if self.details: error_msg = "{} {}".format(error_msg, self.details) + # Note: This else condition can be removed once proposal A from + # b/284179390 is implemented. else: if self.errors: errors = [ diff --git a/google/api_core/extended_operation.py b/google/api_core/extended_operation.py index ca050868..d474632b 100644 --- a/google/api_core/extended_operation.py +++ b/google/api_core/extended_operation.py @@ -158,6 +158,8 @@ def _handle_refreshed_operation(self): return if self.error_code and self.error_message: + # Note: `errors` can be removed once proposal A from + # b/284179390 is implemented. errors = [] if hasattr(self, "error") and hasattr(self.error, "errors"): errors = self.error.errors diff --git a/tests/unit/test_extended_operation.py b/tests/unit/test_extended_operation.py index a850b1a6..e07631d4 100644 --- a/tests/unit/test_extended_operation.py +++ b/tests/unit/test_extended_operation.py @@ -47,6 +47,8 @@ def __init__(self, errors: typing.List[LROCustomError] = []): error_code: typing.Optional[int] = None error_message: typing.Optional[str] = None armor_class: typing.Optional[int] = None + # Note: `error` can be removed once proposal A from + # b/284179390 is implemented. error: typing.Optional[LROCustomErrors] = None # Note: in generated clients, this property must be generated for each From cb9539e914536131f29f1f043a65032184ed5ffc Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Thu, 1 Jun 2023 13:10:11 +0000 Subject: [PATCH 5/6] fix typo --- google/api_core/exceptions.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/google/api_core/exceptions.py b/google/api_core/exceptions.py index 87157d44..d4cb9973 100644 --- a/google/api_core/exceptions.py +++ b/google/api_core/exceptions.py @@ -154,7 +154,8 @@ def __str__(self): for error in self.errors if hasattr(error, "code") and hasattr(error, "message") ] - error_msg = "{} {}".format(error_msg, "\n".join(errors)) + if errors: + error_msg = "{} {}".format(error_msg, "\n".join(errors)) return error_msg @property From 7373ae720a902bb84d6cb364126d4b70055318f2 Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Thu, 1 Jun 2023 14:32:42 +0000 Subject: [PATCH 6/6] fix coverage --- tests/unit/test_extended_operation.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/unit/test_extended_operation.py b/tests/unit/test_extended_operation.py index e07631d4..53af5204 100644 --- a/tests/unit/test_extended_operation.py +++ b/tests/unit/test_extended_operation.py @@ -167,6 +167,24 @@ def test_done_w_retry(): def test_error(): + responses = [ + CustomOperation( + name=TEST_OPERATION_NAME, + status=CustomOperation.StatusCode.DONE, + error_code=400, + error_message="Bad request", + ), + ] + + ex_op, _, _ = make_extended_operation(responses) + + # Defaults to CallError when grpc is not installed + with pytest.raises(exceptions.BadRequest): + ex_op.result() + + # Test GCE custom LRO Error. See b/284179390 + # Note: This test case can be removed once proposal A from + # b/284179390 is implemented. _EXCEPTION_CODE = "INCOMPATIBLE_BACKEND_SERVICES" _EXCEPTION_MESSAGE = "Validation failed for instance group" responses = [