Glance Client in _do_request does not handle 503 (or other HTTP exceptions) in a manageable way.

[John Postlethwait]

On line 514 (and 512) of glance/common/client.py unknown HTTP status codes are raised with an "Unknown error occurred!" using the base python Exception class. This is not a manageable or catchable way of dealing with glance client errors resulting from HTTP error codes...

Any exceptions raised from doing a request should be some sort of HttpException class or something (anything) other than the base Exception class. This will allow consumers of this client to appropriately catch and deal with these exceptions from Glance in a manner that is specific to the sort of error that occurred. HttpException is a vastly different and less severe exception type than just plain-ol' Exception, which is very vague...

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/5366

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/5366
Committed: http://github.com/openstack/glance/commit/c5937221b50614b36d46e77827ab5ad1fd815bd6
Submitter: Jenkins
Branch: master

commit c5937221b50614b36d46e77827ab5ad1fd815bd6
Author: Eoghan Glynn <email address hidden>
Date: Wed Mar 14 11:24:59 2012 +0000

    Well-formed exception types for 413 & 503.

    Fixes bug 952618

    Raise well-formed exception types when 413 or 503
    status returned to client.

    Change-Id: I26118ff7ec7ba968b303435287d0eb3ff4bd443f

[John Postlethwait (john-postlethwait)]

This is a partial fix. While there are manageable error types for new types of errors in this fix, this still has "raise Exception" in the code. All errors from this, especially dealing with HTTP communication, should be some type of Glance exception, not just a core "Exception"

[Eoghan Glynn (eglynn)]

John, can you enumerate the actual status codes you're concerned about?

I didn't think there was much point in polluting the code with new exception types and extra conditional branches for status codes that will never be returned in practice, but if I have missed some, please let me know.

[John Postlethwait (john-postlethwait)]

In practice any time an exception is raised it should be done so using, at least, the base GlanceException class as a means to catch them and do something with them outside of this code.

The code here would cast any unexpected HTTP status code as a base Exception and is thusly difficult to deal with. We encountered this in Horizon the other day with the 503's and ended up having to deal with the exception based not on the exception class itself, but on the actual string error message which is kludgy and difficult.

I'm not proposing extra conditionals, but using a different error class than Exception is really what this bug is about. You shouldn't need to create a new error class to achieve this, using GlanceException would be much more preferable and manageable than using Exception.

[Eoghan Glynn (eglynn)]

John,

The generic GlanceException wouldn't give you much more information, as it stringifies merely as "An unknown exception occurred".

So I've added a new exception type which at least captures the unexpected status code and response body.

Please pitch in to the code review at https://review.openstack.org/5415 if you've any further requirements, so we can this closed off for Essex RC1.

Cheers,
Eoghan

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/5415
Committed: http://github.com/openstack/glance/commit/29b035bcc53a514028e60076692e31a0772b8d5c
Submitter: Jenkins
Branch: master

commit 29b035bcc53a514028e60076692e31a0772b8d5c
Author: Eoghan Glynn <email address hidden>
Date: Thu Mar 15 21:04:56 2012 +0000

    Add new UnexpectedStatus exception.

    Raise UnexpectedStatus as opposed to a generic Exception when
    an unexpected HTTP status is seen by the glance client.

    Further fix for bug 952618

    Change-Id: I222c7553e1a687aec0f6dde8215e4400ea6be2cb