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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add detailed message in job error #1762

Merged
merged 9 commits into from
Jan 10, 2024
Merged

fix: add detailed message in job error #1762

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8278f66
fix: more detailed job error message
Linchin Jan 8, 2024
83470a5
lint
Linchin Jan 8, 2024
e71d0b6
fix mypy error
Linchin Jan 8, 2024
15dd32e
Merge branch 'main' into job_error
chalmerlowe Jan 9, 2024
1b7f668
remove import ignore
Linchin Jan 9, 2024
a777528
Update google/cloud/bigquery/job/base.py
Linchin Jan 9, 2024
57218d2
Update google/cloud/bigquery/job/base.py
Linchin Jan 9, 2024
bb9b134
variable name and unit test
Linchin Jan 9, 2024
dbf004f
Merge branch 'main' into job_error
Linchin Jan 9, 2024
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
21 changes: 18 additions & 3 deletions google/cloud/bigquery/job/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -74,8 +75,20 @@ def _error_result_to_exception(error_result):
status_code = _ERROR_REASON_TO_EXCEPTION.get(
reason, http.client.INTERNAL_SERVER_ERROR
)
# Manually create error message to preserve both error_result and errors.
# Can be removed once b/310544564 and b/318889899 are resolved.
string = ""
chalmerlowe marked this conversation as resolved.
Show resolved Hide resolved
if errors:
string = "; "
for err in errors:
for key, value in err.items():
string += key + ": " + str(value) + ", "
Linchin marked this conversation as resolved.
Show resolved Hide resolved
string = string[:-2]
Linchin marked this conversation as resolved.
Show resolved Hide resolved

error_message = error_result.get("message", "") + string

return exceptions.from_http_status(
status_code, error_result.get("message", ""), errors=[error_result]
status_code, error_message, errors=[error_result]
)


Expand Down Expand Up @@ -886,7 +899,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)
Expand Down
2 changes: 1 addition & 1 deletion samples/snippets/authenticate_service_account.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator

@chalmerlowe chalmerlowe Jan 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@Linchin

Thanks for trying to enable the work to continue moving forward.
I saw an error in the mypy-samples nox session in my PR last night with this line of code.

I worry about this as a "fix" for the problem.
That line of code used to work and doesn't now... not only in this repo, but others so I am not comfortable with just ignoring that line without understanding more about the root cause.

I spoke with Anthonios last night. It appears that there is an issue with the types-protobuf library that causes the mypy-samples error. A change was recently made in that library which broke some things downstream.

I am gonna open a separate PR to set limitations for the types-protobuf module to avoid the problematic version of types-protobuf and provide a comment as to why that version is being avoided.

I am hoping that will fix the issue in a slightly more robust way than simply ignoring the error.

Copy link
Collaborator

@chalmerlowe chalmerlowe Jan 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

PR 1764 was approved. I merged that code into main and into this branch. Hopefully that should clear up the mypy-samples error and we can delete the type: ignore comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for explaining the issue in detail! It was indeed strange, because there wasn't much change in the imported module google.oauth2. I'll remove the ignore here.


# TODO(developer): Set key_path to the path to the service account key
# file.
Expand Down