FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat: add status_code property on http error handling by wmarquardt · Pull Request #1185 · googleapis/google-api-python-client · GitHub

/ google-api-python-client Public

feat: add status_code property on http error handling - #1185

Merged
gcf-merge-on-green[bot] merged 6 commits into
googleapis:masterfrom
wmarquardt:http-error-status
Mar 17, 2021
ConversationCommits (6)ChecksFiles changed
Merged

feat: add status_code property on http error handling#1185
gcf-merge-on-green[bot] merged 6 commits into
googleapis:masterfrom
wmarquardt:http-error-status

Conversation

wmarquardt commented Feb 12, 2021 •
edited by parthea
Loading

Copy link
Copy Markdown
Contributor

Fixes #1183 🦕
Fixes #1255 🦕

  • Add status code property in http error handler class.
  • Resolve issue where error_details is not populated.

wmarquardt requested a review from a team February 12, 2021 21:46
google-cla Bot added the cla: yes This human has signed the Contributor License Agreement. label Feb 12, 2021
wmarquardt mentioned this pull request Feb 12, 2021
Closed
parthea added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 15, 2021
yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 15, 2021
parthea reviewed Mar 3, 2021
View reviewed changes

parthea left a comment •
edited
Loading

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Hi @wmarquardt ,
Thanks for submitting a PR! Could you please also update the getting started documentation here with an example?

https://github.com/googleapis/google-api-python-client/blob/master/docs/start.md#execution-and-response

We could replace the existing example with

try:
    response = request.execute()
except HttpError as e:
    print('Error response status code : {0}, reason : {1}'.format(e.status_code, e.error_details))

parthea commented Mar 3, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

Unrelated to your changes, when I ran the example code I found that e.error_details is empty in my test code. To fix it, I had to make a change in errors.py. The fix was to add a call to self._get_reason() in the __init__ function here.

Change

    @util.positional(3)
    def __init__(self, resp, content, uri=None):
        self.resp = resp
        if not isinstance(content, bytes):
            raise TypeError("HTTP content should be bytes")
        self.content = content
        self.uri = uri
        self.error_details = ""

to

    @util.positional(3)
    def __init__(self, resp, content, uri=None):
        self.resp = resp
        if not isinstance(content, bytes):
            raise TypeError("HTTP content should be bytes")
        self.content = content
        self.uri = uri
        self.error_details = ""
        self._get_reason()

parthea reviewed Mar 3, 2021
View reviewed changes

parthea 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Hi @wmarquardt ,

I missed this in the last review. Please could you also add a docstring in the new function?

Comment thread googleapiclient/errors.py

wmarquardt commented Mar 3, 2021

Copy link
Copy Markdown
Contributor Author

Hi @parthea,

Thank's for your review. I'll make these changes as soon as possible

parthea requested a review from busunkim96 March 17, 2021 16:53
parthea added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 17, 2021
parthea self-assigned this Mar 17, 2021
yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 17, 2021

parthea commented Mar 17, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

@busunkim96 , Please can you take a look? I was able to re-create the issue in #1255 where error_details isn't populated by removing the fix that I added here.

(errors) partheniou@partheniou-vm-1:~/git/google-api-python-client-upstream$ pytest tests/test_errors.py::Error::test_json_body
============================================================================================= test session starts =============================================================================================
...
tests/test_errors.py F                                                                                                                                                                                  [100%]

================================================================================================== FAILURES ===================================================================================================
____________________________________________________________________________________________ Error.test_json_body _____________________________________________________________________________________________

self = <tests.test_errors.Error testMethod=test_json_body>

    def test_json_body(self):
        """Test a nicely formed, expected error response."""
        resp, content = fake_response(
            JSON_ERROR_CONTENT,
            {"status": "400", "content-type": "application/json"},
            reason="Failed",
        )
        error = HttpError(resp, content, uri="http://example.org")
>       self.assertEqual(error.error_details, "error details")
E       AssertionError: '' != 'error details'
E       + error details

tests/test_errors.py:86: AssertionError

busunkim96 reviewed Mar 17, 2021
View reviewed changes

busunkim96 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This looks OK to me @parthea but I am not very familiar with the code in errors.py. Is there something in particular you're concerned about?

parthea added the automerge Merge the pull request once unit tests and other checks pass. label Mar 17, 2021
gcf-merge-on-green Bot merged commit db2a766 into googleapis:master Mar 17, 2021
gcf-merge-on-green Bot removed the automerge Merge the pull request once unit tests and other checks pass. label Mar 17, 2021
gcf-merge-on-green Bot pushed a commit that referenced this pull request Mar 31, 2021
chore: release 2.1.0 (#1256)
cc717a1 
🤖 I have created a release \*beep\* \*boop\*
---
## [2.1.0](https://www.github.com/googleapis/google-api-python-client/compare/v2.0.2...v2.1.0) (2021-03-31)


### Features

* add status_code property on http error handling ([#1185](https://www.github.com/googleapis/google-api-python-client/issues/1185)) ([db2a766](https://www.github.com/googleapis/google-api-python-client/commit/db2a766bbd976742f6ef10d721d8423c8ac9246d))


### Bug Fixes

* Change default of `static_discovery` when `discoveryServiceUrl` set ([#1261](https://www.github.com/googleapis/google-api-python-client/issues/1261)) ([3b4f2e2](https://www.github.com/googleapis/google-api-python-client/commit/3b4f2e243709132b5ca41a3c23853d5067dfb0ab))
* correct api version in oauth-installed.md ([#1258](https://www.github.com/googleapis/google-api-python-client/issues/1258)) ([d1a255f](https://www.github.com/googleapis/google-api-python-client/commit/d1a255fcbeaa36f615cede720692fea2b9f894db))
* fix .close() ([#1231](https://www.github.com/googleapis/google-api-python-client/issues/1231)) ([a9583f7](https://www.github.com/googleapis/google-api-python-client/commit/a9583f712d13c67aa282d14cd30e00999b530d7c))
* Resolve issue where num_retries would have no effect ([#1244](https://www.github.com/googleapis/google-api-python-client/issues/1244)) ([c518472](https://www.github.com/googleapis/google-api-python-client/commit/c518472e836c32ba2ff5e8480ab5a7643f722d46))


### Documentation

* Distinguish between public/private docs in 2.0 guide ([#1226](https://www.github.com/googleapis/google-api-python-client/issues/1226)) ([a6f1706](https://www.github.com/googleapis/google-api-python-client/commit/a6f17066caf6e911b7e94e8feab52fa3af2def1b))
* Update README to promote cloud client libraries ([#1252](https://www.github.com/googleapis/google-api-python-client/issues/1252)) ([22807c9](https://www.github.com/googleapis/google-api-python-client/commit/22807c92ce754ff3d60f240ec5c38de50c5b654b))
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
dermasmid mentioned this pull request Sep 2, 2021
Merged
gcf-merge-on-green Bot pushed a commit that referenced this pull request Sep 2, 2021
fix: remove repeated calls to self._get_reason (#1513)
d5cf4e0 
self._get_reason is being called in \_\_init\_\_ (#1185)  , so why not save it then?
also in the \_\_repr\_\_ function we got the reason by calling the _get_reason function right in the beginning, but was then called again.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

parthea parthea approved these changes
+1 more reviewer
busunkim96 busunkim96 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

parthea

wmarquardt

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

HttpError error_details isn't populated unless __repr__ is called first Get HTTP status code error handling

4 participants

Back | FazBrowse Home | New Git URL