After moving the replace_strings() call into __init__, I think I'm going
to move it back to the start of execute(). With the call in __init__, I
get output that looks like this:
14:49:05 Build datasets successfully setup
14:49:05 Simple log:
/rpool/cr_7036694/dc/ai/logs/simple-log.2011-04-14.14:49
14:49:05 Detail Log:
/rpool/cr_7036694/dc/ai/logs/detail-log.2011-04-14.14:49
14:49:11 Custom Script provided is: '/usr/bin/cp /etc/system
{PKG_IMAGE_PATH}/HEY'
14:49:11 Custom Script to run is: '/usr/bin/cp /etc/system
/rpool/cr_7036694/dc/ai/build_data/pkg_image/HEY'
14:49:13 Custom Script provided is: '/usr/bin/cp /etc/system
%{//[@solaris_install.target.logical.Filesystem#1].mountpoint}/build_data/boot_archive/SYSTEM'
14:49:13 Custom Script to run is: '/usr/bin/cp /etc/system
/rpool/cr_7036694/dc/ai//build_data/boot_archive/SYSTEM'
14:49:14 === Executing Custom Script Checkpoint ===
14:49:30 === Executing Custom Script Checkpoint ===
I don't want to move the "=== Executing ...." string into the __init__
method as that would be inconsistent with the rest of the DC checkpoints.
Since the checkpoint is so simplistic, I'd prefer to keep the call to
replace_strings() in execute() to keep all the output consistent with
the other checkpoints.
Are you ok with this?
-Drew
On 4/14/11 2:46 PM, Drew Fisher wrote:
Sure. I'll move the call into __init__().
Thanks for the review!
-Drew
On 4/14/11 2:14 PM, Karen Tung wrote:
Hi Drew,
In addition to Keith's comment, I want to add the following.
Should we call "self.replace_strings()" at the __init__() function?
Since we got all the information we need there to do the string
replacement, why delay it? Furtheremore, if the command they
specify is None, it's better to know sooner rather than later.
Thanks,
--Karen
On 04/14/11 12:52, Keith Mitchell wrote:
Seems like 91-97 will generate duplicate output, since you're using
the inherent logging of solaris_install.Popen.
Other than that, looks fine.
On 04/14/11 12:12 PM, Drew Fisher wrote:
Good afternoon!
Could I please get a code review for the following bug:
http://monaco.us.oracle.com/detail.jsf?cr=7036694
The webrev is here:
http://cr.opensolaris.org/~drewfish/cr_7036694/
The crux of the problem is the custom_script checkpoint has no way
of translating the new {PKG_IMAGE_PATH} and {BOOT_ARCHIVE}
replacement strings to their respective directories. This fix
allows users to specify those replacement strings in the checkpoint
args:
<checkpoint name="custom-script-one"
desc="custom script one"
mod_path="solaris_install/distro_const/checkpoints/custom_script"
checkpoint_class="CustomScript">
<args>/usr/bin/cp /etc/system {PKG_IMAGE_PATH}/myfile</args>
</checkpoint>
Also, the original replacement still works, so people can continue
using:
<checkpoint name="custom-script-two"
desc="custom script two"
mod_path="solaris_install/distro_const/checkpoints/custom_script"
checkpoint_class="CustomScript">
<args>/usr/bin/cp /etc/system
%{//[@solaris_install.target.logical.Filesystem#1].mountpoint}/build_data/boot_archive/SYSTEM</args>
</checkpoint>
I prefer the first method. :)
In addition to updating the checkpoint for support of these
replacement strings, I moved to using solaris_install.Popen, and
cleaned up some extra code that served no purpose in the original.
Thanks!
-Drew
_______________________________________________
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
