Hi Karen,
The document looks good - but as I'm sure you expect there are some comments...
I've not mentioned anything specific to the use of the DOC for checkpoints, I
think that we've enough of a discussion going on in the other threads on
this... :)
Comments:
Section 3, page 5 :
DataObjectCache should really read "Data Object Cache" to refer to the
overall project rather than the specific class...
Section 4, page 5:
I'm wondering if the programming language to use is really a choice for
this project since it's been decided at the higher level of the CUD design
that Python is the language of choice for new implementations.
Section 5.2, page 6: (and in some way Sections 6.5 and 10)
The "get_progress_estimate()" method could do with some definition of what
should be returned here - is it a time estimate (seconds or minutes)? I
certainly think that it would be useful to have an example of how this
should be used by developers - I realise section 10 provides some idea of
what this is, but probably useful to spell out an example, and probably
best done in section 10 to also provide an example of normalizing.
The provision of a "cancel()" method implies that things are
multi-threaded - I'm wondering at what level threading is being done, is
it at the Application level, the Engine level, or the Checkpoint level.
If threading is done by the Engine - i.e. all checkpoints are run in a
thread, one after the other, does this imply that the "execute()" method
is asynchronous, in that it will return immediately and not wait for
execution of the checkpoints to be completed - if this is the case a
notification mechanism to let you know when things are totally completed
would be useful - I know progress kinda gives you this, but it would be no
harm to have a "completed" call-back.
I think a section on threading would be useful, since I believe it has an
impact on other elements of the install infrastructure.
Section 6.1, page 7:
The DOC is not a singleton any more, and there will be a need of a
"get_data_object_cache()" method on the Engine.
Section 6.3.1, page 8:
Maybe the use of the term "dangerous" is too severe? It might be better to
be specific about the risk, and how it might be able to be prevented.
Nit: last paragraph : "The user is allow to" -> "The user is allowed to"
Section 6.3.2, page 8 (and some others like Section 7.4.1)
Nit: The references to checkpoints "a, b, c..." - it would be useful to
somehow make these stand out from the rest of the text (capitalize, bold,
italics, etc - or use a specific style like "Source Text").
Section 6.3.4, page 8:
register_checkpoint() parameter "checkpoint_obj", would be more correctly
known as "checkpoint_class_name".
Section 6.7.1, page 12:
How are you going to guarantee that removal of snapshots from /tmp will
not remove snapshots from other processes - e.g. are you using a dir for
each run (/tmp/install_snapshots.PID/), just might be worth spelling out.
Section 7.3.2.2, page 14:
Nit: s/termites/terminates/
Section 7.5, page 15 & 16:
How is "DataObjectCacheNotEmpty" state determined? Should the DOC provide
a is_empty() method is or just checking that len(doc.children) == 0,
enough?
Thanks,
Darren.
On 05/28/10 11:12 PM, Karen Tung wrote:
> Hi,
>
> The draft install engine design doc is ready for review.
> I pushed the doc to the caiman-docs repo. You can
> find them under the install_engine directory in
> ssh://[email protected]/hg/caiman/caiman-docs
>
> If you don't want to clone the gate, you can also access
> the file directly here:
>
> http://cvs.opensolaris.org/source/xref/caiman/caiman-docs/install_engine
>
> You will find 2 files in the directory:
> * engine-design-doc.odt is the design document
> * engine-sequence-diag-0528.png is the sequence diagram inserted into
> chapter 14 of the document. You might want to see the bigger image
> directly.
>
> Please send your feedback by 6/11/2010.
>
> If you plan to review the document, please send me an email privately,
> and preferably also let me know when you will complete the review, so
> I can plan accordingly and ensure the document is reviewed thoroughly.
>
> Thanks,
>
> --Karen
> _______________________________________________
> caiman-discuss mailing list
> [email protected]
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
