[katello-devel] Foreman integration model question

[Martin Bacovsky]

On 08/22/2012 07:07 PM, Justin Sherrill wrote:
> On 08/22/2012 08:26 AM, Petr Chalupa wrote:
>> Lukas is right, our motivation was to break the code into smaller 
>> pieces (I am big fan of SOLID and 'single responsibility principle' 
>> in particular). If you write code this way, you can then move things 
>> around very quickly so you can develop features and fix bugs faster. 
>> No to mention it can be testes more easily.
>>
>> app/models/user.rb (User) is representation of katello user which 
>> mixes (if foreman is installed) foreman orchestration 
>> app/models/glue/forema/user.rb. Orchestration is working with foreman 
>> through children of ForemanModel which are representation of remote 
>> resources app/models/foreman/user.rb (Foreman::User). Foreman::User 
>> is using foreman-api bindings which are defined as singletons in 
>> lib/resources/foreman.rb.
>>
>> So there are 4 layers:
>> - katello model
>> - foreman-katello orchestration
>> - foreman model (remote resource)
>> - foreman bindings (gem foreman-api)
>>
>> I also for splitting up big parts.
> I do like this auto generated api method better (aside from there 
> being two distinct User models).  To say a downside of the current 
> model is a giant single file pulp.rb is kinda moot, as that was just 
> how it was organized, we can organize it differently quite easily.  
> What we are doing here with foreman is wayyyy more than just 
> organizing it differently, its a fairly completely different way of 
> orchestrating things, bypassing our lazy_accessor magic.
>
> For example, lets say we do the same with environment, where there is 
> a katello environment and a foreman environment.  And lets say there 
> is some special piece of information only stored in foreman (this is 
> not a far-fetched scenario).
>
> With our current orchestration you would do something like:
>
> a = KTEnvironment.find(3)
> print a.remote_attribute
> a.remote_attribute = "foo"
> a.save!
>
> whereas the way i see it now, we'd have to do:
>
> a = KTEnvironment.find(3)
> a.foreman_user.remote_attribute = "foo"
> a.save!
>
> I don't really like this better as you'd have to remember which 
> backend engine a particular attribute is.  Today you don't.  I don't 
> really expect any of these arguments to change the way we are doing 
> anything, its just annoying to change our entire way of working with a 
> backend engine without any discussion :)
>
> We need to be aware that doing it this way is creating a 2nd way with 
> working with fully (or partially) remote objects and that needs to be 
> clear and apparent.  It also needs to be justified :)
>
> -Justin
>

Good point with the attributes. On the other hand the approach Peter 
used for Foreman model feels to me a way better. From my experience the 
lazy-stuff usually translates as "easy at writing busy at maintaining" ;).

Would it be solution to keep the current implementation and add a way to 
access the foreman specific attributes form the katello model? Perhaps 
by explicitly including the attributes handling to katello model from 
the glue layer?

Martin





>>
>> Petr
>>
>> On 22.08.12 10:19, Lukas Zapletal wrote:
>>> On Tue, Aug 21, 2012 at 02:48:56PM -0400, Justin Sherrill wrote:
>>>> Taking user for example we would have:
>>>>
>>>> user.rb
>>>> glue/pulp/user.rb
>>>
>>> But you are missing one piece of the puzzle:
>>>
>>> lib/resources/pulp.rb (class User)
>>>
>>> And that is the piece which you can find in foreman/user.rb.
>>>
>>>> What I see with foreman is:
>>>>
>>>> user.rb
>>>> glue/foreman/user.rb
>>>> foreman/user.rb
>>>
>>> So instead having all the resources in a single class
>>> (lib/resources/foreman.rb), we are going to have (or better - generate)
>>> all resource (read also as "dumb") classes. Each in separate file.
>>>
>>>> Where foreman/user is a second class that can be instantiated and
>>>> used on its own, while glue/foreman/user.rb is simply the
>>>> orchestration to create that object.  I see the use of
>>>> Resources::ForemanModel, but in this user instance the foreman user
>>>> could be manipulated completely apart from the katello user (which
>>>> is bad IMHO).  This also is completely different from our existing
>>>> orchestration methods.
>>>
>>> Yes, it is different. I like this approach more. Creating a one 
>>> instance
>>> per request is nothing, while the code is definitely better than eight
>>> hundred lines long pulp.rb with all the resources there as "class"
>>> methods.
>>>
>>> I think this is the correct approach and we may discuss splitting those
>>> big resource files in future into smaller pieces for other backend
>>> engines too. If we write a similar generator, we wont longer need to
>>> write them manually (and we will be able to track API changes too).
>>>
>>> I was not writing the code, but I believe those instances are treated
>>> like singletons, so you do not need to create instances by yourself:
>>>
>>> https://github.com/Katello/katello/blob/foreman_architectures/src/lib/resources/foreman.rb 
>>>
>>>
>>> Guys please elaborate what the intentions are.
>>>
>>
>> _______________________________________________
>> katello-devel mailing list
>> katello-devel at redhat.com
>> https://www.redhat.com/mailman/listinfo/katello-devel
>
> _______________________________________________
> katello-devel mailing list
> katello-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/katello-devel