Hi Alok,
Everything is looking really good to my eye. I have just a few last
minute nits and minor questions.
General nit: Some of the added logging statements are uncapitalized. I
don't know if that bothers anyone besides me though.
distro_const/__init__.py:
135: keyword argument defaults should be static, due to a quirk in how
they work. So, baseargs should default to "None", and then, in the first
line of the function, be overridden to "sys.argv[1:]" if it's been set
to None. (Probably not a big deal for this particular function, since it
likely won't be run twice in the same process, and the variable is only
read, not returned or modified)
grub_setup.py:
381: Nit: Add a note in the docstring that this is a text install
specific function?
pkg_img_mod.py
159-163: Can you confirm that this is still actually needed and desired,
per bug 14559?
pre_pkg_img_mod.py:
217: Does LD_LIBRARY_PATH need to be unset here, or is the comment out
of date?
install_utils.py:
set_http_proxy: If the http_proxy environment variable is already set
(e.g., the user ran something like: "HTTP_PROXY=http://example.com
distro_const build manifest.xml"), should that take precedence?
Thanks,
Keith
On 11/18/10 02:27 PM, Alok Aggarwal wrote:
Thanks to everyone who reviewed the code for the
DC->CUD project the first time around. Here's round two
that incorporates most of the changes brought up.
Incremental webrev against the initial webrev:
http://cr.opensolaris.org/~aalok/cud_dc-diff2/
Complete webrev:
http://cr.opensolaris.org/~aalok/cud_dc-2/
Some of the most significant changes included here
are:
- DC Logging has been overhauled. logger's transfer_log()
was enhanced as part of CR 7000990 and DC now uses that
instead of managing log files, etc by itself.
- Determining the ZFS mountpoints: This code was reworked
to utilize the zfs code in install_target. The mountpoints
are now determined *after* TI has executed instead of trying
to determine mountpoints ahead of time.
- DC Lockfile: This is now a context manager (used via the
"with" statement) and has added additional streamlining
- Unittests: There are now unittests for the primary DC
application. Coverage is ~ 70%
- A SimpleSchemaElement class has been defined for use by
.. well, simple schema elements. This code will eventually
be moved to the DOC code (there's an outstanding bug on this).
- kwargs/args in execution_checkpoint.py is now a separate class
- Sort files are now included in the source
- Manifest Parser tests that were commented out due to the absence
of DC in slim_source have been uncommented.
Please take a look and provide your feedback by COB, December 1.
Thanks,
Drew and Alok
_______________________________________________
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
