[PATCH conductor] Image building compatible with ActiveResource 3.2

Jason Guiditta jguiditt at redhat.com
Mon Aug 20 16:05:48 UTC 2012


On 20/08/12 17:52 +0200, jstransk at redhat.com wrote:
>From: Jiri Stransky <jstransk at redhat.com>
>
>Right now we have ActiveResource 3.2 in our Gemfile.lock, but Conductor
>was not compatible with it. Building images did not work. This commit
>fixes it.
>
>When doing:
>
>    image = Aeolus::Image::Factory::Image.new(:id => some_id)
>    image.save!
>
>ActiveResource 3.0 would make a PUT request to
>.../images/some_id
>but ActiveResource >= 3.1 would make a POST request to
>.../images
>
>This caused miscommunication between Conductor and ImageFactory when
>using ActiveResource 3.2. The image that we wanted to build would remain
>untouched (without any build or target image linked to it), but a new,
>redundant image would be created and built. After this commit everything
>works fine.
>
>Unfortunately, there does not seem to be a way to fix this without
>explicitly asking for ActiveResource version (as long as we want to
>support both ActiveResource 3.0 and 3.2 at the same time). After
>ActiveResource 3.0 is obsoleted from Conductor, we can remove the
>version check and use the code for 3.2.

I am not against checking for the version temporarily, we have that in
a few places already.  However, I think it may make for less
duplication (and more robust support) to put this check in our client
library, aeolus-image.  Then we can leave the conductor codebase alone
when we toss this in favor of the new rails engine martyn has been
working on.  I also suspect that while this is only causing a few
tests to fail, the same appraoch is needed for other calls to ARes
that may not be well tested.  IOW, the tests may pass, but I bet we
still end up with unexplained errors when doing other things with
Factory of iwhd.  So, my suggestion is to make a small module in
aeolus-image and mix it into the base classes for the factory and
warehouse models.

-j



More information about the aeolus-devel mailing list