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] Fixed auth exception reporting and improved exception reporting itself. (JIRA: #DTACLOUD-29)
Date Thu, 31 Mar 2011 00:08:24 GMT
On Wed, 2011-03-30 at 14:46 +0200, mfojtik@redhat.com wrote:
> From: Michal Fojtik <mfojtik@redhat.com>
> 
> ---
>  server/lib/deltacloud/base_driver/base_driver.rb   |   31 ++++++++++++++------
>  server/lib/deltacloud/drivers/ec2/ec2_driver.rb    |    4 +-
>  .../lib/deltacloud/drivers/gogrid/gogrid_driver.rb |    9 ------
>  server/lib/deltacloud/drivers/mock/mock_driver.rb  |    4 +-
>  .../drivers/rackspace/rackspace_driver.rb          |   12 ++++----
>  server/server.rb                                   |    3 +-
>  6 files changed, 34 insertions(+), 29 deletions(-)

ACK. There are some puts in 'safely' that seem like leftovers from
debugging. Attached is your patch with them removed. I'd rather commit
that.

The whole error processing machinery seems both too fancy and not
flexible enough; it might be better to get rid of the
caught_exceptions_list and just let drivers define callbacks similar in
spirit to Rails' error callbacks.

This could be switched by the class of the exception, so that the EC2
driver could say

        on_error Aws::AwsError do |e|
          if e.http_code == 401
            raise Deltacloud::AuthException.new
          else
            raise Deltacloud::BackendError.new(502, e.class.to_s, e.message, e.backtrace)
          end
        end

The other thing that pains me of course is that we don't have unit tests
for this. I've been playing with mocking out the place in the EC2 driver
that calls 'describe_instances' to throw an Aws::AwsError with status
401, but haven't gotten that to work.

In any event, the two issues above should only be addressed after the
next release.

David


Mime
View raw message