Dave,
Again, thanks!! Hopefully, with the comments the review
was easier.
Comments below.
Thanks,
John
Dave Miner wrote:
> installadm.h. 54: nit, align expansion with neighboring lines
Absolutely. This was an issue with my editor that changed the
indent for tab.
> delete_service.py
>
> 187: the design for the INETd_CONF functionality is flawed (sigh), but
> that's not your fault; I've filed bug 13123 to cover this issue.
Thanks.
> 205, 218: stylistically, I'd think it makes more sense to put the
> comment inside the except block than outside.
Moved the comment.
> 222ff: This logic isn't quite the same as what in.tftpd uses when -s is
> specified (which is the normal and recommended case). If -s is used and
> the directory doesn't exist the the tftp service should go to
> maintenance. I think you can simplify on this basis.
I am going to assume that this will be fixed when defect 13123 is resolved.
> list.py
>
> 211: s/ot/to/
Corrected.
> 246: seems that you should just use socket.gethostyname_ex() here
That would be shorter (because I no longer would need an additional
function) and easier.
> 311: don't grok this comment...
Ok.
> 316: should we be using getfqdn() instead?
Yes. Will use it instead.
> 322: this comment could be construed that you're creating a directory,
> might rephrase
Ah, didn't mean that at all and I see how it could be construed that
way. Will reword.
# get the Network IP path for the host
> 379: this comment seems incorrect; /etc/netboot doesn't appear to be
> used anywhere here (correctly)
Yeap. That's what I get for working late Friday night. I've updated
the comment appropriately.
We first get the TFTProot directory. Then iterate over the
files within the TFTProot directory to get the client menu
which contains the Image Path and Service Name. The client
and image path are then added to the named service dictionary
list.
> 560: how would this condition be the case? Should we do something
> stronger than just skip it?
It shouldn't happen at all. If it does then there is something wrong
within the SMF service. I will send a message to stderr and exit. I
have code elsewhere as well and will treat the issue the same way.
> 916: Seems like we can get some sort of traceback out of this operation,
> as I don't see except clauses that will catch a failure anywhere further
> up the chain.
I missed that call to AIdb.DB and another set of AIdb calls. Will wrap
with try/except.
> Dave
