deltacloud-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "marios@redhat.com" <mandr...@redhat.com>
Subject Re: [PATCH core 2/2] CIMI: Added more syntax sugar for add_collection_member
Date Thu, 06 Sep 2012 07:00:35 GMT
On 06/09/12 02:58, David Lutterkort wrote:
> On Wed, 2012-09-05 at 15:15 +0200, mfojtik@redhat.com wrote:
>> From: Michal Fojtik <mfojtik@redhat.com>
>>
>> * This patch also moves this to CIMI::Model::Base instead
>>   of schema to make sure the updated schema is the model
>>   schema.
>>
>> Signed-off-by: Michal fojtik <mfojtik@redhat.com>
>> diff --git a/server/lib/cimi/models/base.rb b/server/lib/cimi/models/base.rb
>> index b375ce0..b627b82 100644
>> --- a/server/lib/cimi/models/base.rb
>> +++ b/server/lib/cimi/models/base.rb
>> @@ -94,6 +94,20 @@ class CIMI::Model::Base
>>    # attribute, we also define a getter and a setter to access/change the
>>    # value for that attribute
>>    class << self
>> +
>> +    def <<(model)
>> +      clone_base_schema unless base_schema_cloned?
>> +      member_name = model.name.split("::").last
>> +      if ::Struct.const_defined?("CIMI_#{member_name}")
>> +        puts "Removing struct"
>> +        ::Struct.send(:remove_const, "CIMI_#{member_name}")
>> +      end
>> +      member_symbol = member_name.underscore.pluralize.to_sym
>> +      members = CIMI::Model::Schema::Array.new(member_symbol)
>> +      members.struct.schema.attributes = model.schema.attributes
>> +      base_schema.attributes << members
>> +    end
> 
> I have to admit that I don't understand this code at all; how does the
> assignment members.struct.schema.attributes = model.schema.attributes
> make things safer ? Also, why can't the same be done in the Schema
> class, since we only manipulate attributes of the Schema class in the
> code above ?

some clarification. This:

>> +      member_symbol = member_name.underscore.pluralize.to_sym
>> +      members = CIMI::Model::Schema::Array.new(member_symbol)
>> +      members.struct.schema.attributes = model.schema.attributes
>> +      base_schema.attributes << members

is the code I wrote to add a 'collection member array' for each of the
collections. It's not part of 'making things safer' (the four lines
above that are to do with preventing overriding of the struct) and it
*was* is Schema originally (as I'm writing this its still in schema.rb
line 242).

The idea was to 'implement collections' as quickly as possible with what
we have... this is the quickest way I could think of. So for example:

 16 class CIMI::Model::MachineCollection < CIMI::Model::Base$
 17 $
 18   act_as_root_entity :machine$
 19 $
 20   text :count$
 21 $
 22   #add machines array:$
 23   self.schema.add_collection_member_array(CIMI::Model::Machine)

^^^line 23 here calls those four lines above. Make a new schema array
for 'machines', add the attributes of the CIMI::Model::Machine schema to
this and then add this array to the CIMI::Model::MachineCollection
schema... so that:

 25   def self.default(context)$
 26     machines = CIMI::Model::Machine.all(context)$
 27     self.new($
 28       :id => context.machines_url,$
 29       :name => 'default',$
 30       :created => Time.now,$
 31       :description => "#{context.driver.name.capitalize}
MachineCollection",$
 32       :count => machines.size,$
 33       :machines => machines$
 34     )$
 35   end$

when we add the list of machines on line 33 they get serialized
correctly and according to the CIMI::Model::Machine schema.

> 
> In general, we should be able to get rid of all the *Collection classes
> - that was part of the whole collection exercise in CIMI, to make it
> possible to use the same code for all collections.
> 
> In terms of the schema DSL, I would strive towards something that lets
> us describe machines like so:
> 
> class CIMI::Model::Machine < CIMI::Model::Base
>   
>         # Set up plumbing for the machine collection, i.e., code
>         # to serialize/deserialize a collection of machines
>         acts_as_root_collection
> 
>         text :state
>         text :cpu
>         ...
>         
>         # Expand to a collection with count, operations, etc.
>         collection :disks, :class => CIMI::Model::Disk
> end
> 
> I think we will need to have collections as a subclass of
> Schema::Attribute at some point, so that we can control expansion of
> collections via $expand during the serialization phase. Basically,
> collections have two serializations: the unexpanded one with just a
> href, and the expanded one with all the details. Which means that at
> some point the to_xml/to_json methods will have to take parameters to
> control expansion.
> 

This looks neat and will make serialization/deserialization of
*sub*-collections much easier. However I'm not clear on how you think we
should handle the collections themselves... I mean like
MachineCollection vs MachineVolumeCollection (perhaps we're overloading
'collection' - I think of them in terms of collections and
sub-collections). Is what we're doing now (i.e. the
add_collection_member_array in schema.rb which Michal has moved to '<<'
in base.rb) ok?

marios

> I don't think we should address $expand complications with these
> patches, but that's where this code is headed.
> 
> David
> 
> 


Mime
View raw message