[Ovirt-devel] [PATCH server] Revised attempt at refactoring the vm_controller using the service layer.

[Scott Seago]

Ian Main wrote:
> On Wed, 22 Apr 2009 12:35:55 -0400
> Scott Seago <sseago at redhat.com> wrote:
>
>   
>> Ian Main wrote:
>>     
>>> On Tue, 21 Apr 2009 21:49:15 +0000
>>> David Lutterkort <lutter at redhat.com> wrote:
>>>
>>>   
>>>       
>>>> On Tue, 2009-04-21 at 13:14 -0700, Ian Main wrote:
>>>>     
>>>>         
>>>>> So with this service layer.. with it being included and setting local
>>>>> properties of the object, will that be thread safe?  Or will we have
>>>>> to do locking around each call?
>>>>>
>>>>> I'm not sure yet if the agent will be threaded.. but it certainly
>>>>> could have a thread per method invocation..
>>>>>       
>>>>>           
>>>> To echo what Scott said: I would prefer if we followed the rails model
>>>> fairly closely, with some sort of notion of a 'QMF controller', and
>>>> instantiate one for every method call from the QMF agent.
>>>>
>>>> Given that we want to minimize the amount of things we keep in memory
>>>> (mostly to avoid cache coherency issues between memory and DB), we want
>>>> the QMF API to be as statefree as possible.
>>>>     
>>>>         
>>> Yeah that makes sense.. the only place where it might be a little odd is
>>> in setting properties.  When properties are set it seems the way to do it
>>> is to load the vm properties hash from an active record and then call
>>> update() to have everything verified the same way as in rails?
>>>
>>> I'm not sure if there's any other way to verify the contents of various
>>> fields using the active record directly?  Either way it seems we need
>>> direct acccess to it as well.. is that a problem?
>>>
>>>     Ian
>>>
>>>   
>>>       
>> Yes, to make persistent changes to VM properties here you would need to 
>> use the svc_update call. Or are you saying you also want to make changes 
>> to activerecord objects outside the service API layer? This would 
>> certainly be possible -- and if you call save! then any validation 
>> errors will raise exceptions just like they do in the svc method, 
>> although you'd lose the permission checks and any other business logic 
>> that is in the service layer.
>>     
>
> I think in many instances it would be easier to simply change the active record directly.  If you think about something like..
>
> vm = ovirt.create_vm_def(...)
> vm.description = 'changed description'
>
> in the backend there will be a call to the 'description' method of a
> VmDef object.  So in ruby we'll have eg:
>
> class VmDef
>
>   def description(new_desc)
>     vm = vm_ar.reload
>     vm.description = new_desc
>     vm.save!
>   end
>
> ...
>
> Otherwise you have to load the VM active record and make a hash from it
> (?? maybe that's easy?) and then call svc_update() with the new hash..
> maybe it would be possible to just have the permission check callable
> directly?  Not sure what is best..
>
>     Ian
>
>   
For edits you only need to include changed attributes in the hash. So 
you could call svc_obj.svc_update(vm_id, {:description => 'changed 
description'}).

I'm still slightly confused on how VmDef relates to the service classes. 
Is VmDef the class that includes the VmService module? Or is VmDef 
something longer-lived and more related to the activerecord stuff directly?

In any case whatever class includes VmService will have access to the 
activerecord VM ID somehow so it could call svc_update(vm_id, 
{:description => 'changed description'}) if all you want to do is change 
the description. Something like this:

  def description(new_desc)
    #svc_update handles saving here
    begin
      # replace vm_ar.id with whatever means we have to get the id for the current object.
      svc_update(vm_ar.id,:description => new_desc)
    rescue
      # error handling here
    end
  end


The biggest sticking point here would probably not be the mechanics of 
what methods we implement, etc -- but the fact that the expected scope 
of any object including VmService is handling a single user request. If 
we need a single in-memory object to span multiple requests you'd 
probably need to go to the activerecord model directly (and then have to 
deal with the various synchronization/threading/locking issues).

Scott
> _______________________________________________
> Ovirt-devel mailing list
> Ovirt-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/ovirt-devel
>