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.

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

Fixes #937 - Adds HTTP method and URL to exceptions. #938

Closed
wants to merge 1 commit into from

Conversation

jgeewax
Copy link
Contributor

@jgeewax jgeewax commented Jun 23, 2015

Example:

Old: gcloud.exceptions.NotFound: 404 Resource not found (resource=new).

New: gcloud.exceptions.NotFound: 404 Resource not found (resource=new). (DELETE https://pubsub.googleapis.com/v1beta2/projects/jjg-cloud-research/topics/new)


Wasn't sure about a couple things, so wanted to ask...

  1. Name of the variable (request_string). Seems expressive, but would rather pass a request object or something similar. Should I rename? Or pass two (method and url) on to make_exception?
  2. Assembling the request string in the make_exception call (seemed like it might be better done on a separate line?)

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 23, 2015
@@ -391,7 +391,8 @@ def api_request(self, method, path, query_params=None,
target_object=_target_object)

if not 200 <= response.status < 300:
raise make_exception(response, content)
raise make_exception(response, content,
request_string=method+' '+url)

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@dhermes
Copy link
Contributor

dhermes commented Jun 25, 2015

Let's not let this thing linger. Merge or close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: core cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants