From 8278f66f0369cca49dc4fe787ae1f31b8a2f6673 Mon Sep 17 00:00:00 2001 From: Linchin Date: Mon, 8 Jan 2024 21:18:55 +0000 Subject: [PATCH 1/7] fix: more detailed job error message --- google/cloud/bigquery/job/base.py | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/google/cloud/bigquery/job/base.py b/google/cloud/bigquery/job/base.py index 97e0ea3bd..a3a110a0c 100644 --- a/google/cloud/bigquery/job/base.py +++ b/google/cloud/bigquery/job/base.py @@ -55,7 +55,7 @@ } -def _error_result_to_exception(error_result): +def _error_result_to_exception(error_result, errors=None): """Maps BigQuery error reasons to an exception. The reasons and their matching HTTP status codes are documented on @@ -66,6 +66,7 @@ def _error_result_to_exception(error_result): Args: error_result (Mapping[str, str]): The error result from BigQuery. + errors (Union[Iterable[str], None]): The detailed error messages. Returns: google.cloud.exceptions.GoogleAPICallError: The mapped exception. @@ -74,9 +75,20 @@ def _error_result_to_exception(error_result): status_code = _ERROR_REASON_TO_EXCEPTION.get( reason, http.client.INTERNAL_SERVER_ERROR ) - return exceptions.from_http_status( - status_code, error_result.get("message", ""), errors=[error_result] - ) + # Manually create error message to preserve both error_result and errors. + # Can be removed once b/310544564 and b/318889899 are resolved. + string = "" + if errors: + string = "; " + for err in errors: + for key, value in err.items(): + string += key + ": " + str(value) + ", " + string = string[:-2] + + error_message = error_result.get("message", "") + string + + return exceptions.from_http_status(status_code, error_message, errors=[error_result]) + ReservationUsage = namedtuple("ReservationUsage", "name slot_ms") @@ -886,7 +898,9 @@ def _set_future_result(self): return if self.error_result is not None: - exception = _error_result_to_exception(self.error_result) + exception = _error_result_to_exception( + self.error_result, self.errors or () + ) self.set_exception(exception) else: self.set_result(self) From 83470a585198d620f57e0ba8f095e3521b42ac22 Mon Sep 17 00:00:00 2001 From: Linchin Date: Mon, 8 Jan 2024 22:17:19 +0000 Subject: [PATCH 2/7] lint --- google/cloud/bigquery/job/base.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/google/cloud/bigquery/job/base.py b/google/cloud/bigquery/job/base.py index a3a110a0c..223500948 100644 --- a/google/cloud/bigquery/job/base.py +++ b/google/cloud/bigquery/job/base.py @@ -78,7 +78,7 @@ def _error_result_to_exception(error_result, errors=None): # Manually create error message to preserve both error_result and errors. # Can be removed once b/310544564 and b/318889899 are resolved. string = "" - if errors: + if errors: string = "; " for err in errors: for key, value in err.items(): @@ -87,8 +87,9 @@ def _error_result_to_exception(error_result, errors=None): error_message = error_result.get("message", "") + string - return exceptions.from_http_status(status_code, error_message, errors=[error_result]) - + return exceptions.from_http_status( + status_code, error_message, errors=[error_result] + ) ReservationUsage = namedtuple("ReservationUsage", "name slot_ms") From e71d0b6eead763b13f0e9cbf1bd2f27f58e73d59 Mon Sep 17 00:00:00 2001 From: Linchin Date: Mon, 8 Jan 2024 23:25:22 +0000 Subject: [PATCH 3/7] fix mypy error --- samples/snippets/authenticate_service_account.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/snippets/authenticate_service_account.py b/samples/snippets/authenticate_service_account.py index 8a8c9557d..24840fc1c 100644 --- a/samples/snippets/authenticate_service_account.py +++ b/samples/snippets/authenticate_service_account.py @@ -24,7 +24,7 @@ def main() -> "bigquery.Client": # [START bigquery_client_json_credentials] from google.cloud import bigquery - from google.oauth2 import service_account + from google.oauth2 import service_account # type: ignore # TODO(developer): Set key_path to the path to the service account key # file. From 1b7f668256f028b2b61ca21405c179c3fe17c83b Mon Sep 17 00:00:00 2001 From: Lingqing Gan Date: Tue, 9 Jan 2024 10:59:44 -0800 Subject: [PATCH 4/7] remove import ignore --- samples/snippets/authenticate_service_account.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/snippets/authenticate_service_account.py b/samples/snippets/authenticate_service_account.py index 24840fc1c..8a8c9557d 100644 --- a/samples/snippets/authenticate_service_account.py +++ b/samples/snippets/authenticate_service_account.py @@ -24,7 +24,7 @@ def main() -> "bigquery.Client": # [START bigquery_client_json_credentials] from google.cloud import bigquery - from google.oauth2 import service_account # type: ignore + from google.oauth2 import service_account # TODO(developer): Set key_path to the path to the service account key # file. From a777528af19f843c4fd7ba140d462c034aaa9271 Mon Sep 17 00:00:00 2001 From: Lingqing Gan Date: Tue, 9 Jan 2024 12:56:47 -0800 Subject: [PATCH 5/7] Update google/cloud/bigquery/job/base.py Co-authored-by: Chalmer Lowe --- google/cloud/bigquery/job/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/cloud/bigquery/job/base.py b/google/cloud/bigquery/job/base.py index 223500948..216fa11c3 100644 --- a/google/cloud/bigquery/job/base.py +++ b/google/cloud/bigquery/job/base.py @@ -82,7 +82,7 @@ def _error_result_to_exception(error_result, errors=None): string = "; " for err in errors: for key, value in err.items(): - string += key + ": " + str(value) + ", " + string += f"{key}: {value}, " string = string[:-2] error_message = error_result.get("message", "") + string From 57218d2e7863d406789c0640d56b12df9239c7a7 Mon Sep 17 00:00:00 2001 From: Lingqing Gan Date: Tue, 9 Jan 2024 12:57:18 -0800 Subject: [PATCH 6/7] Update google/cloud/bigquery/job/base.py Co-authored-by: Chalmer Lowe --- google/cloud/bigquery/job/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/cloud/bigquery/job/base.py b/google/cloud/bigquery/job/base.py index 216fa11c3..75119e157 100644 --- a/google/cloud/bigquery/job/base.py +++ b/google/cloud/bigquery/job/base.py @@ -83,7 +83,7 @@ def _error_result_to_exception(error_result, errors=None): for err in errors: for key, value in err.items(): string += f"{key}: {value}, " - string = string[:-2] + string = string[:-2] # strips off the last unneeded comma and space error_message = error_result.get("message", "") + string From bb9b134abcf721bab933be18c40a489beb458490 Mon Sep 17 00:00:00 2001 From: Linchin Date: Tue, 9 Jan 2024 21:51:56 +0000 Subject: [PATCH 7/7] variable name and unit test --- google/cloud/bigquery/job/base.py | 16 ++++++++++------ tests/unit/job/test_base.py | 21 +++++++++++++++++++++ 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/google/cloud/bigquery/job/base.py b/google/cloud/bigquery/job/base.py index 75119e157..2641afea8 100644 --- a/google/cloud/bigquery/job/base.py +++ b/google/cloud/bigquery/job/base.py @@ -77,15 +77,19 @@ def _error_result_to_exception(error_result, errors=None): ) # Manually create error message to preserve both error_result and errors. # Can be removed once b/310544564 and b/318889899 are resolved. - string = "" + concatenated_errors = "" if errors: - string = "; " + concatenated_errors = "; " for err in errors: - for key, value in err.items(): - string += f"{key}: {value}, " - string = string[:-2] # strips off the last unneeded comma and space + concatenated_errors += ", ".join( + [f"{key}: {value}" for key, value in err.items()] + ) + concatenated_errors += "; " + + # strips off the last unneeded semicolon and space + concatenated_errors = concatenated_errors[:-2] - error_message = error_result.get("message", "") + string + error_message = error_result.get("message", "") + concatenated_errors return exceptions.from_http_status( status_code, error_message, errors=[error_result] diff --git a/tests/unit/job/test_base.py b/tests/unit/job/test_base.py index 5635d0e32..a61fd3198 100644 --- a/tests/unit/job/test_base.py +++ b/tests/unit/job/test_base.py @@ -47,6 +47,27 @@ def test_missing_reason(self): exception = self._call_fut(error_result) self.assertEqual(exception.code, http.client.INTERNAL_SERVER_ERROR) + def test_contatenate_errors(self): + # Added test for b/310544564 and b/318889899. + # Ensures that error messages from both error_result and errors are + # present in the exception raised. + + error_result = { + "reason": "invalid1", + "message": "error message 1", + } + errors = [ + {"reason": "invalid2", "message": "error message 2"}, + {"reason": "invalid3", "message": "error message 3"}, + ] + + exception = self._call_fut(error_result, errors) + self.assertEqual( + exception.message, + "error message 1; reason: invalid2, message: error message 2; " + "reason: invalid3, message: error message 3", + ) + class Test_JobReference(unittest.TestCase): JOB_ID = "job-id"