Also I just realised that this returns a 503. 

I'm wondering whether this should be a 502.  I do find the HTTP specification difficult to interpret sometimes.  But I think from the definitions below.  A 502 would be a better fit:


Taken from HTTP 1.1 Specification.

10.5.3 502 Bad Gateway

The server, while acting as a gateway or proxy, received an invalid response from the upstream server it accessed in attempting to fulfill the request.

10.5.4 503 Service Unavailable

The server is currently unable to handle the request due to a temporary overloading or maintenance of the server. The implication is that this is a temporary condition which will be alleviated after some delay. If known, the length of the delay MAY be indicated in a Retry-After header. If no Retry-After is given, the client SHOULD handle the response as it would for a 500 response.

      Note: The existence of the 503 status code does not imply that a
      server must use it when becoming over





On 09/06/2012 02:15 PM, Martyn Taylor wrote:
I've tested this manually and it works fine.

Can you please add tests for both this patch and its conductor counterpart.  Also can you change the error message from: 'imagefactory is dead, Jim!'  to something a bit better ;)

Your free to push with these changes in.

Cheers

Martyn


On 03/15/2012 03:51 PM, mzatko@redhat.com wrote:
From: Maros Zatko <mzatko@redhat.com>

https://bugzilla.redhat.com/show_bug.cgi?id=801527

API:
* new API exception ServiceUnavailable
* rescue for ECONNREFUSED when imagefactory is down
---
  src/app/controllers/api/images_controller.rb |   14 +++++++++-----
  src/lib/exceptions.rb                        |    1 +
  2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/src/app/controllers/api/images_controller.rb b/src/app/controllers/api/images_controller.rb
index 465a20b..2bcbeee 100644
--- a/src/app/controllers/api/images_controller.rb
+++ b/src/app/controllers/api/images_controller.rb
@@ -89,11 +89,15 @@ module Api
              :template => @tpl.uuid,
              :environment => @pool_family.name
            })
-          @image = Aeolus::Image::Factory::Image.new(:id => iwhd_image.id)
-          @image.targets = req[:params][:targets]
-          @image.template = req[:params][:template]
-          @image.save!
-          respond_with(@image)
+          begin
+            @image = Aeolus::Image::Factory::Image.new(:id => iwhd_image.id)
+            @image.targets = req[:params][:targets]
+            @image.template = req[:params][:template]
+            @image.save!
+            respond_with(@image)
+          rescue Errno::ECONNREFUSED
+            raise(Aeolus::Conductor::API::ServiceUnavailable.new(503, 'Imagefactory is dead, Jim!'))
+          end
          elsif req[:type] == :import
            account = ProviderAccount.find_by_label(req[:params][:provider_account_name])
            raise(Aeolus::Conductor::API::ProviderAccountNotFound.new(404, t("api.error_messages.provider_account_not_found",
diff --git a/src/lib/exceptions.rb b/src/lib/exceptions.rb
index d56ddb3..9b1edab 100644
--- a/src/lib/exceptions.rb
+++ b/src/lib/exceptions.rb
@@ -38,6 +38,7 @@ module Aeolus
        class ProviderImageDeleteFailure < Error; end
        class ProviderImageNotFound < Error; end
        class ProviderImageStatusNotFound < Error; end
+      class ServiceUnavailable < Error; end
        class TargetImageDeleteFailure < Error; end
        class TargetImageNotFound < Error; end
        class TargetImageStatusNotFound < Error; end