Michael,
fixup_content is to simply make sure that the serialized xml will
confirm to what DMTF spec requires. and it also makes sure that the format
is good to serialize to json which also needs to be confirmed to the spec.
fixup_content method does not operate on xml, it operates on the hash
object as the comments indicated.
When you drill deeper into dmtf spec, you will find most of the
operations will require xml or json document, that is, most of the requests
post xml or json documents, in current DC implementation, post is rarely
used , that means, majority of the request only deal with request
parameters, but when you start dealing with DMTF requests, most likely the
request contains xml or json document, the very first thing that you have
to do is to parse the xml or json. What the current CIMI code does is to
treat the default data and the request data exactly the same, there won't
be any extra code needed when handling more complex request. You can remove
them now but you will eventually have to add xml/json handling to parse the
data posted by requests, that will end up having two set of the code. I
will be very hesitate to approve what patches 3 to 8 do.
XmlSimple seems to me a very good xml document handling library. I do
not feel either it being complex or hard to understand. I feel that it is
miles miles easier to be understood than the DC code itself. If I had not
had David hand holding me on DC code and its framework, I won't be able to
understand how DC works even today. The XmlSiple library has been ported
to many different programming languages and very well accepted in Ruby
community as far as I can see. As I said, you will have to eventually deal
with XML/JSON input. Of course, this is just my 2 cents. If there is other
even better and easier alternatives for handling xml input, I'll be very
glad to try it out.
Thanks.
Tong Li
Emerging Technologies & Standards
B062/K317
liton...@us.ibm.com
From: mfoj...@redhat.com
To: deltacloud-dev@incubator.apache.org
Date: 10/27/2011 07:17 AM
Subject: Second revamp of CIMI code
Hi,
This patchset should improve the code we have currently for CIMI.
I will briefly explain what each patch did and why:
1/8:
This is just cosmetic change. It's better to have consistent DSL
rather than using 'global!' method inside all collections.
If some collection want be 'global' lets call it 'global_collection'.
This change was tested on Ruby 1.8 and 1.9 and should not break anything.
2/8:
Using EOS is a bit weird to me. We used EOL just because our Deltacloud
descriptions was too long to fit into one line ;-)
However the CIMI descriptions are short one liners so why not put them
into regular string.
3-6/8:
The code in cmwgapp_helper was too much complex to me to undestand
(and read) so I decided to rewrite it. You can now see the difference.
I dunno how about you, but I had very hard time to undestand what
'fixup_content' or 'respond_to_collection' methods internally do.
We can discuss it, but my personal opinion is that hacking XML using
XmlSimple and then modifying resulting Hash is too complex.
7/8:
The code previosly used to return collection was using XmlSimple
and not HAML. This throw an Exception in Ruby 1.8:
NoMethodError - undefined method `downcase' for 77:Fixnum:
./bin/../lib/cimi/helpers/cmwgapp_helper.rb:68:in `respond_to_collection'
This is one of the examples of darkside XmlSimple. The code is too
complex and thus there is higher propability of exceptions like this one.
I spend 2 hours debugging the code until I finally discover from where
this problem original came.
The 'index.xml'haml' files are easy readable, we can update them according
to CIMI everytime and everyone can undestand what should be returned on
output
just by looking on HAML file. Later on, we can re-use this index.xml.haml
as we did in Deltacloud and use 'partial' rendering to render items in
collection.
8/8:
I had long conversation about this with Tong Li and as far I undestand
his point of using this variable is to make a bridge between Deltacloud API
object model and CIMI. However I think we can easely change our object
to support CIMI specific properties very easely without breaking anything.
Since every class is open we can simple 're-open' the model class (like
Instance) and add more methods here.
My personal opinion is that as this implementation will grow and we will
need to add more data and properties, this bridge will become our
nightmare.
-----
IMPORTANT: This patch is not addressing the UI. The HAML views will need to
be updated to support new local variables and stuff.
Especially 'new' operation views, will need to be updated
together
with our models. I'll take this task once we will have basic
Test::Unit ready (to be sure I'll not break anything).
PS: This patchset was tested under Ruby 1.8 and 1.9 (and Rubinius for
fun ;-).
-- Michal
