Hi Keith,
On 01/ 7/11 02:14 PM, Keith Mitchell wrote:
On 01/ 7/11 02:01 PM, Karen Tung wrote:
Hi Keith,
grub_setup.py:
----------------------
lines 185-191: "installadm_entry" is a new optional argument you
introduced.
Why not explicitly checking for that argument since that's the only
thing
that gets interpreted at this time? The way it is coded now will allow
other arguments to be specified, but then, people will be confused as
to why the values they specified are being ignored.
The call to __setup will reject any arguments that aren't
"installadm_entry". I'm trying to let Python do the checking for me.
(Note that other checkpoints *do* run into the problem you've
mentioned - see PkgImgMod for example. Perhaps a CR should be filed to
adjust some of this handling - since it would also be nice to be able
to specify the keyword args directly in __init__, rather than wrapping
them in arg - I can't recall if there was a reason for doing it as
written though.)
OK, now, I understand why you did it that way.
I know that the engine allows one to specify keyword args directly in
__init__.
In fact, if you do that, the engine will even validate that they
provided all
the expected args at registration time.
Take a look at usr/src/lib/install_engine/test/empty_checkpoint.py.
It shows you all the combination of args and keyword arg scenarios.
Looks like it is DC's choice to pass in the args like that. Perhaps Alok
or Drew know why. I agree with you that other checkpoints do run into
the problem of arguments not being completely validated. Let me file a
bug on that.
pkg_img_mod.py
------------------------
- line 381: Why did you add a call to create_save_list() here? My
understanding
is that the file created by the create_save_list() call will be used as
"instructions" on what files to download from the server when the
text installer
boots up. I don't think this file is needed for the case of the LiveCD.
Removed.
net-fs-root
---------------
- line 380: Nit: I was confused for a little bit when I read this
line, because you specified
the output file first, which is different convention than the rest of
the wget calls in the file where
you first specify which file to download, and then, specify where to
put it. Perhaps it will
be good to be consistent?
Specifying flag/options after the argument list seems odd in general,
but for consistency I'll switch it.
Since the changes from above are simple, I won't generate an updated
webrev, unless you'd like to see one.
No need to generate another webrev just for me. :-)
Thanks,
--Karen
- Keith
Thanks,
--Karen
On 01/ 6/11 03:57 PM, Keith Mitchell wrote:
This might help:
http://cr.opensolaris.org/~kemitche/6971585.final/
On 01/ 6/11 03:56 PM, Keith Mitchell wrote:
Hi,
Can I get one or two sets of eyes to look at (what I hope to be)
the last webrev for netboot text installer? This work was started
by Andrew and Chris last fall, and I've merged it with the updated
DC, as well as ironed out a few last minute bugs (plus tossed in a
fix to 7010312 for good measure).
The files that have changed since last are:
grub_setup.py
pkg_img_mod.py
net-fs-root
ti_install.py
Though comments on the other files are of course welcome as well.
Thanks,
Keith
_______________________________________________
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
