Skip to content
This repository was archived by the owner on Feb 23, 2026. It is now read-only.

feat: add support for 'error_info' - #307

Closed
atulep wants to merge 7 commits into
googleapis:mainfrom
atulep:feat_error_details2
Closed

feat: add support for 'error_info'#307
atulep wants to merge 7 commits into
googleapis:mainfrom
atulep:feat_error_details2

Conversation

@atulep

Copy link
Copy Markdown
Contributor

Adds error_info field to GoogleAPICallError, as requested in #286.

@atulep
atulep requested review from a team and summer-ji-engNovember 8, 2021 21:32
@google-clagoogle-claBot added the cla: yes This human has signed the Contributor License Agreement. label Nov 8, 2021
@tseavertseaver changed the title feat: Adds support for error_info.feat: add support for 'error_info'Nov 9, 2021

@tseavertseaver left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All returns from _parse_grpc_error_details must return a 2-tuple.

Comment threadgoogle/api_core/exceptions.py Outdated
Co-authored-by: Tres Seaver <tseaver@palladion.com>
@google-cla

google-claBot commented Nov 9, 2021

Copy link
Copy Markdown

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@google-clagoogle-claBot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Nov 9, 2021
@google-cla

google-claBot commented Nov 9, 2021

Copy link
Copy Markdown

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@google-cla

google-claBot commented Nov 9, 2021

Copy link
Copy Markdown

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@tseaver

Copy link
Copy Markdown
Contributor

@googlebot I consent.

@google-cla

google-claBot commented Nov 9, 2021

Copy link
Copy Markdown

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@tseaver

Copy link
Copy Markdown
Contributor

@atulep The CLA bot has (once again) lost its tiny mind. You'll need to remove the cla: no label and add the cla: yes label manually.

@atulep

Copy link
Copy Markdown
ContributorAuthor

@atulep The CLA bot has (once again) lost its tiny mind. You'll need to remove the cla: no label and add the cla: yes label manually.

Hey @tseaver, it seems to complain because email used in the commit "@palladion.com" is different than the one you used to sign CLA with. The docs suggest "If the email address does not match an email found, please ask the contributor to either add their new email address to their CLA or rebase their commits with their correct email address." Can you please do that?

Comment threadgoogle/api_core/exceptions.py

@summer-ji-engsummer-ji-eng left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Left one comment, other than that, looks good to me. :)

@tseaver

Copy link
Copy Markdown
Contributor

@atulep

it seems to complain because email used in the commit "@palladion.com" is different than the one you used to sign CLA with.

The error message is incorrect -- all my commits in the googleapis / GoogleCloudPlatform organzations for the past seven years have been signed with that e-mail address, which is also the only one associated with my Github profile. Sometimes (I don't know that pattern) the bot balks at merged through-the-web suggestions (which is the case here).

Comment threadtests/unit/test_exceptions.py
Comment threadgoogle/api_core/exceptions.py
@atulep

Copy link
Copy Markdown
ContributorAuthor

@atulep

it seems to complain because email used in the commit "@palladion.com" is different than the one you used to sign CLA with.

The error message is incorrect -- all my commits in the googleapis / GoogleCloudPlatform organzations for the past seven years have been signed with that e-mail address, which is also the only one associated with my Github profile. Sometimes (I don't know that pattern) the bot balks at merged through-the-web suggestions (which is the case here).

I checked the internal dashboards, and it shows you signed CLA with a "@gmail.com" address. This is the official instruction I was told to follow:

One of the most common problems is that the git author email in the commit is not an email address associated with a CLA. The solution is to change the git author email to be an address covered by the CLA. That email should also be added to their GitHub account; it doesn't need to be the primary email, but it should be on the account.

I know you're an active contributor, so this is really weird to see an CLA issue. Can you share a similar issue from the past where a Googler manually removed the CLA label? Thank you.

@tseaver

Copy link
Copy Markdown
Contributor

@tswast, @busunkim96 Can one of you please tag in on the manual CLA flip?

@tswasttswast added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Nov 10, 2021
@google-clagoogle-claBot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Nov 10, 2021
@vam-google

Copy link
Copy Markdown
Contributor

@googlebot I fixed it.

@atulepatulep added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Nov 10, 2021
@google-clagoogle-claBot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Nov 15, 2021
@atulepatulep added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Nov 15, 2021
@google-clagoogle-claBot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Nov 15, 2021
@atulep

Copy link
Copy Markdown
ContributorAuthor

@summer-ji-eng@tseaver - finally addressed your feedback. Thanks for your comments!

@tseaver
tseaver dismissed summer-ji-eng’s stale reviewNovember 16, 2021 18:19

Requested changes made in 331d6e3

@tseaver

Copy link
Copy Markdown
Contributor

@atulep the CLA flag has to be cleared again after pushing any commit, even a merge.

@summer-ji-engsummer-ji-eng left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚀

@atulepatulep added the cla: yes This human has signed the Contributor License Agreement. label Nov 17, 2021
@google-clagoogle-claBot removed the cla: no This human has *not* signed the Contributor License Agreement. label Nov 17, 2021
@tswasttswast added cla: yes This human has signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Nov 17, 2021
@atulep

Copy link
Copy Markdown
ContributorAuthor

@tseaver I manually flipped the flag but it still blocks me from merging.

@software-dovsoftware-dov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, just some nitpicking.

Comment on lines +161 to +163
if not self._error_info:
return None
return self._error_info.reason

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: we can simplify this to

returnself._error_info.reasonifself._error_infoelseNone

Comment on lines +175 to +177
if not self._error_info:
return None
return self._error_info.domain

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here: we can use conditional expressions

Comment on lines +189 to +191
if not self._error_info:
return None
return self._error_info.metadata

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

And here, same as above.

Comment on lines +481 to +487
error_info = list(
filter(
lambda detail: detail.get("@type", "")
== "type.googleapis.com/google.rpc.ErrorInfo",
details,
)
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: it's more idiomatic to use list comprehensions than list and filter.

error_info_type="type.googleapis.com/google.rpc.ErrorInfo"error_info= [dfordindetailsifd.get("@type", "") ==error_info_type]

@atulep

Copy link
Copy Markdown
ContributorAuthor

Closing this in favor of #315.

@atulepatulep closed this Nov 29, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yesThis human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@atulep@tseaver@vam-google@software-dov@summer-ji-eng@tswast