LGTM.
-Drew
On 3/5/12 9:50 AM, John Fischer wrote:
Darren and Drew,
I've updated the webrev:
https://cr.opensolaris.org/action/browse/caiman/johnfisc/7097012-AI-output-console-2
https://cr.opensolaris.org/action/browse/caiman/johnfisc/7097012-AI-output-console-diff
The CR may be viewed at:
7097012 AI should output to /dev/console as well as
/system/volatile/install_log
http://monaco.us.oracle.com/detail.jsf?cr=7097012
Thanks,
John
On 03/ 5/12 06:18 AM, John Fischer wrote:
Thanks Drew and Darren!! I'll leave them in and add a comment
at the beginning of the imports.
John
Sent from my iPad
On Mar 5, 2012, at 1:31 AM, Darren Kenny<[email protected]>
wrote:
On 03/03/2012 01:27, Drew Fisher wrote:
John,
7041360 is, I think why you can't remove those imports.
From the CR:
"If we change code to load the checkpoints with the 'dotted'
version of
the path, the imp.find_module() will only
work if we've already imported that module explicitly in to our code."
I know that Darren can confirm if this is the reason those imports are
there. Don't remove them yet, please.
This is exactly why those imports are there - if you run with the
engine in
debug mode it isn't possible to catch any exceptions from these modules
using specific names due to the above bug.
Now, we have changed things so that we're no longer running the
engine in
debug mode so things will probably run without them now, but I feel
it's
worth having these around should we have any need to since later it
would
be very confusing.
Possible a comment is worth while here instead of removing - they
will be
imported anyway later by the engine...
Thanks,
Darren.
-Drew
On 3/2/12 6:21 PM, John Fischer wrote:
Drew,
I have removed them. I am now working on creating a new image
to test. Now if only the servers we reachable.
John
Sent from my iPad
On Mar 2, 2012, at 4:39 PM, Drew Fisher<[email protected]>
wrote:
I don't think we can remove those. I remember that this has
something to do with the engine and how the checkpoints are
registered.
Darren: does this ring a bell to you?
-Drew
On 3/2/12 12:33 PM, Mary Ding wrote:
John:
There were also a lot of unused imports in auto_install.py
according to pylint and it will be nice if they can be fixed.
http://indiana-build.us.oracle.com/job/slim_code_cleanliness/Pylint_Audit/?
Mainly it is W0611 and the following imports are unused:
Unused import boot
Unused import device_config
Unused import boot_archive
Unused import instantiation
Unused import initialize_smf
Unused import transfer_files
Unused import apply_sysconfig
Unused import INSTALL
Unused import update_dumpadm
Unused import DataObject
Unused import discovery
Unused import varshared
Unused import setup_swap
Unused import create_snapshot
Unused import TargetSelectionZone
On 03/02/12 11:28, Mary Ding wrote:
John:
My comments are just nits:
In auto_install.py, if you use pylint 0.23, it will complain
about the following lines:
import os
import os.path
W0404 Reimport 'os.path' (imported line 10)
It will be good if you can fix it.
On 03/01/12 17:37, John Fischer wrote:
All,
Can I get a code review for:
7097012 AI should output to /dev/console as well as
/system/volatile/install_log
http://monaco.us.oracle.com/detail.jsf?cr=7097012
The webrev is located at:
https://cr.opensolaris.org/action/browse/caiman/johnfisc/7097012-AI-output-console/
The basic fix is to have all checkpoints produce output. This
is done by passing in
a kwargs show_output to engine.register_checkpoint(). In
addition to this fix I have
cleaned up the output for AI changing several things from
debug to info and visa versa.
Furthermore, there was some duplicate output during the early
stages due to fractional
percents. This was fixed by saving the previous non-time
progress and comparing it
with the current non-time progress.
I have built an AI iso image using distro_const, setup and
installed a new host. Ethan
checked on a zones setup. I also ran the test suite. All
testing was as expected. The
code is pep8 and pylint clean, improved or unchanged.
Thanks,
John
_______________________________________________
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
_______________________________________________
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
_______________________________________________
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
