Hi Karen,
Thanks for the feedback, comments/responses below...
On 05/21/10 11:35 PM, Karen Tung wrote:
> On 05/19/10 07:34 AM, Darren Kenny wrote:
>
> Hi Darren and Dermot,
>
> Very well written and thorough document. I have a few comments.
>
> General comment:
> --------------------------------
>
> There's no discussion throughout the document on how any error from
> interacting with the DOC will be reporting, ie: what kind of exception
> will be raised..etc..
> Also, there is no discussion in the document about what, if anything,
> messages
> will be logged to help with debugging..etc..
True, that is an omission, I'll see what I can come up with....
It's not really expected that there will actually be all that many errors.
Most of the get methods will return None if nothing matches the parameters.
Adding children isn't expected to be troublesome, but if we do enforce a
uniqueness in the name of a child of any given parent (as mentioned in the
e-mail from Dave Miner) then we may throw an exception if the name isn't
unique.
Similarly in asked to insert before/after another child then we may
also throw an exception if that child doesn't exist...
One case where a failure might be appropriate is if you attempt to add a child
to an object where that child has already been added to another object (i.e.
child.parent != None and child.parent != new_parent).
The from_xml() methods aren't expected to fail, failure should be discovered
in the can_handle() method first in the majority of cases, but we should
probably require an exception to terminate the from_xml importing if invalid
data if found.
to_xml() should never fail, other than returning None if no XML is to be
generated.
>
> Specific comments:
> ---------------------------------
>
> Page 4, section 1.1, 5th paragraph...
> "....writing this to a permanent store such as a file is done by the
> ManifestParser, but the DOC coordinates this process."
>
> I understand you want to express the idea that DOC will not be writing
> the XML manifest. However, I think it is confusing to specially call out
> ManifestParser as the component that will do the work. To me,
> the name ManifestParser suggests that it is the program that will
> parse the manifest. It is confusing to say that a parser will write the
> content too. Would it be better to mention that some other component will
> write the XML?
I think that's a good point, and I'll make the change.
>
> page 9, section 3.4.1.1
>
> The description of "parent" property says that it is a read-only property by
> consumers. In the tree manipulation section,section 3.4.1.2, none of
> the append child API allow you to specify a parent object. How do
> I create the tree structure?
The parent is the object that you call add_child() on, so if you do :
myobject1.add_child( myobject2 )
then myobject1 is the parent of the child myobject2. The parent property in
myobject2 will be set by this call to be myobject1, such that the following
would then be true:
myobject2.parent == myobject1
So the root of the tree will be the DataObjectCache object, which will always
have a parent == None.
>
> Page 9, section 3.4.1.2
>
> I think it would be helpful to have some psedo-code somewhere creating
> a tree with some fake objects using these APIs. I think that will
> really help in understanding how these API can be used to build and
> manipulate
> the tree.
Understood, examples are certainly useful, I'll try come up with some.
>
> Page 24, Use Case #2
>
> I am completely confused by this example, it seems to contradict
> with the current engine design in quiet a few areas.
> Below is my understanding of use case #2.
> I want to make sure we are on the same page before I make further comments.
>
> My understanding of use case #2 is that information about each of the
> checkpoints the application wants to execute will be store in
> ExecutionCheckpoints objects. The application will create these
> objects and add them to the DOC.
Not quite, there is only 1 ExectutionCheckpoints object. It then contains, as
children, the Checkpoints[1], which are objects that describe a checkpoint,
it's parameters, and any data required to create the real checkpoint.
So you would have a tree in the DOC something like:
DOC
ExecutionCheckpoints
Checkpoints sub-class (e.g. TD)
Checkpoints sub-class (e.g. TI)
...
Checkpoints sub-class (e.g. TransferCPIO)
Checkpoints sub-class (e.g. TransferIPS)
The UML diagram on page 25 attempts to describe this.
> When the application calls execute(), the engine search the DOC for
> the list of un-executed ExecutionCheckpoint objects,
> and initialize the checkpoints and execute them. Therefore, we will
> *not* need to have the engine.register_checkpoing() functionality??
That depends on how the Engine implements it - this is only a use-case, and
one possible solution.
The Engine design could still prefer to use the register_checkpoints() method,
which would make the Engine the one to do the insertion in to the DOC, taking
the place of the Application in terms of the manipulation of the DOC in this
use-case.
Either is possible really, but the hope is that the end-result would be the
same.
This use-case is based on the recent discussions with Jean and Sarah
One of the reasons for it being like this is that the ManifestParser
functionality, for DC at least, may insert Checkpoints in to the cache based
on the contents of the XML (e.g. finalizers).
But it's expected that this wouldn't be the complete list of Checkpoints, and
the Application would then, after loading the Manifest into the DOC, insert
it's own check "always required, but not written to manifest" Checkpoints
using the insert_before/after methods.
An example of this might be:
- ManifestParser imported the Manifest, and got:
DOC
ExecutionCheckpoints
TransferIPS - User's IPS packages
Finalizer1 - User's Script 1
Finalizer2 - User's Script 2
Finalizer3 - User's Script 3
- DC then add's it's always present checkpoints:
DOC
ExecutionCheckpoints
+ TargetDiscovery - Limited to zpool discovery only
+ TargetInstantiation - Create Staging Areas (DC zpools)
+ TransferIPS - Populate Staging Area
+ TransferCPIO - Copy from Staging Area - /
+ TransferCPIO - Copy from Staging Area - /usr
TransferIPS - User's IPS packages
Finalizer1 - User's Script 1
Finalizer2 - User's Script 2
Finalizer3 - User's Script 3
+ ICT_VBox - Create VBox Image
+ ICT_ISO - Create ISO Image
If we were to only allow addition using register_checkpoints() this might not
be possible.
> The application will register the checkpoints they want to execute into
> the DOC directly, instead of going through the engine.
>
> The above is my understanding of reading use case #2. Is this what you
> meant?
Certainly this is what I described as a possible use-case of the DOC - but as
I say it's *not* intended to be the final result, consumers of the API can
certainly do what they want, but this only attempts to provide a solution that
would work with the DOC as designed.
The actual Engine design would need to be discussed in it's own right...
Thanks,
Darren.
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
