[katello-devel] some thoughts on foreman-architectures branch

[Hugh Brock]

On Wed, Sep 05, 2012 at 04:30:59PM +0100, Dmitri Dolguikh wrote:
> The logic used to access Foreman server is spread over several
> classes: a bunch of automatically-generated proxy-type classes that
> reside in foreman_api gem, ForemanModel (resides in
> lib/resources/foreman_model.rb), that is a base-class for Foreman
> resource classes that reside in app/models/foreman.
> 
> Good things first:
>  - app/models/foreman classes are minimal, and clearly show what
> attributes are being used.
>  - app/models/foreman classes allow for local validation via
> standard ActiveModel validators.
>  - app/models/foreman classes allow for easy customization of json
> generation.
> 
> 
> Things I don't like:
>  - I can't shake the feeling that the approach represented in the
> branch is a huge duplication of effort. We are replicating
> ActiveResource on every step of the way, and:
>   - ActiveResource's error handling is better/consistent (ours is
> minimal and current approach would require us to recreate all of
> RestClient's exceptions)
>   - ActiveResource's error parsing is much better (we'll have to
> duplicate AR's code to be able to parse remote errors).
>   - Pretty much all of ForemanModel is what we'd get with
> ActiveResource for free. Keep in mind that custom methods are simple
> with ActiveResource.
>  - Foreman uses rabl [1] for json-generation, we implemented a
> custom approach. I think rabl is at least worth a look.
>  - Apipie in it's current shape promotes code duplication. See
> foreman_api classes, domain- and architectures-controllers in
> katello.
> 
> 
> I propose:
>  - It's been close to a couple of years since we looked at
> ActiveResource last. I'd like to see where it is now, how hard it is
> to use with oauth and non-rails types of resource urls these days.
>  - I'd like to evaluate rabl for use in Katello
>  - I'd like to fix code duplication issues in Apipie, possibly
> switch it to use ActiveResource instead of RestClient
> 
> 
> -d
> 
> 
> [1] https://github.com/nesquena/rabl <https://github.com/nesquena/rabl>

Absent any knowledge of the actual code I strongly agree :).

Seriously, we are using active resource pretty heavily in a lot of
places in Conductor, I would like to see us standardize on it to the
extent we can.

--Hugh


-- 
== Hugh Brock, hbrock at redhat.com                                   ==
== Engineering Manager, Cloud BU                                   ==
== Aeolus Project: Manage virtual infrastructure across clouds.    ==
== http://aeolusproject.org                                        ==

"I know that you believe you understand what you think I said, but I’m
not sure you realize that what you heard is not what I meant."
--Robert McCloskey