10:51 a.m.
From: Maros Zatko <mzatko(a)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
--
1.7.7.6
8:15 a.m.
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(a)redhat.com wrote:
From: Maros Zatko <mzatko(a)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
8:49 a.m.
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(a)redhat.com wrote:
> From: Maros Zatko <mzatko(a)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
4247
days inactive
4422
days old
aeolus-devel@lists.fedorahosted.org
2 comments
2 participants
participants (2)
-
Maros Zatko -
Martyn Taylor