Sent to the wrong alias the first time.

Resending ...

====
Clay,

I'm going to focus on the Python changes here.

delete_service.py
=======
483-499:  can we un-camelcase the menuLst variable?  menu_lst would be
better (and pep8 compliant)

ai_sd.py
=======

General Nit:  For all of your subprocess calls, you should dump the
exception checking of OSError and ValueError.  By definition those are
fatal errors and should propagate up.  This has the added benefit of
slimming your code down as you're not constantly re-writing the same
code over and over.

http://docs.python.org/library/subprocess.html#exceptions


36, 38-39:  alphabetize your imports

511-519:  can you not do try/except/else/finally?

try:
    f = open(self.wanboot_conf, "r")
except IOError, err:
    yield None, err
else:
    yield f, None
finally:
    f.close()

instead of the nested try/finally?


561-564, 576:  wrap these in a try/except so if wanboot.conf ever changes,
you can exit cleanly.  I've always found that specifying the index I
want after a split will somehow come back and bite me so I've had to
learn (the hard way) of wrapping ANY calls like this in try/except so
I can handle the errors correctly.

580-586:  indentation mistake?  Why are these not indented under the
class?

655-658:  be consistent with the commands above.  Pass Popen() a list

cmd = ["/sbin/dhcpinfo", "BootSrvA"]
try:
    dhcpinfo = subprocess.Popen(cmd, stdout=PIPE, stderr=PIPE)

671:  change to check dhcpinfo.returncode.  Not everything spits out
to stderr

676-680:  combine to a single return line as you do on line 578

return dhcp_stdout.strip()


======

-Drew



On 07/30/10 05:10, [email protected] wrote:
Hi all,
If you have provided me feedback on the AI Multi-homed feature code, could you please ensure your desired changes have been affected? I would ideally like a go/no-go by COB Monday 8/2. Also, for everyone's information, it was requested that I add a feature to allow a multi-homed server to masquerade as a single-homed server. This allows support for older multi-homed unaware images to be served on a multi-homed server with only some loss of multi-homed functionality. Lastly, a number of QE found bugs have been addressed as well.

Full webrev:
http://cr.opensolaris.org/~clayb/webrev_multihomed//final_full/
Differential from initial webrev:
http://cr.opensolaris.org/~clayb/webrev_multihomed//final_diff/
Original webrev:
http://cr.opensolaris.org/~clayb/webrev_multihomed/

                            Thank you,
                            Clay
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

Reply via email to