deltacloud-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Lutterkort <lut...@redhat.com>
Subject Re: [PATCH core 2/3] Client: Revamped client/server error handling
Date Mon, 27 Feb 2012 23:30:29 GMT
On Mon, 2012-02-27 at 16:48 +0100, mfojtik@redhat.com wrote:
> From: Michal Fojtik <mfojtik@redhat.com>
> 
> This patch will various HTTP error exceptions to
> client library. The exception classes are then used
> to properly report server/client exceptions and they
> should help handle various HTTP errors.

ACK, with a couple nits:

> Signed-off-by: Michal fojtik <mfojtik@redhat.com>
> ---
>  client/lib/deltacloud.rb         |   72 ++++++++------------
>  client/lib/errors.rb             |  140 ++++++++++++++++++++++++++++++++++++++

This is more of a longstanding problem, than something about htis patch:
we are very unclean with the namespace of our client gem; we should
really have everything under lib/deltacloud/ for the client to comply
with Ruby conventions.

>  client/specs/errors_spec.rb      |   59 ++++++++++++++++
>  server/views/errors/500.xml.haml |    2 +
>  server/views/errors/501.xml.haml |   13 +---
>  server/views/errors/502.xml.haml |   13 +---
>  server/views/errors/504.xml.haml |   13 +---

The changes to the error templates for 502 and 504 should really be
folded into patch 1/3; that probably means that the changes to 500 and
501 also need to go into that patch, or into a separate one preceding
1/3.

> diff --git a/client/lib/deltacloud.rb b/client/lib/deltacloud.rb
> index 614eab2..a52af1f 100644
> --- a/client/lib/deltacloud.rb
> +++ b/client/lib/deltacloud.rb

> @@ -349,6 +348,28 @@ module DeltaCloud
>        headers
>      end
>  
> +    def response_successfull?(code)

There's a typo in successful: it should only have one l.

David



Mime
View raw message