Karen Tung wrote: > Hi Jean, > > Here are my comments: > > transfer_mod.py: > > - line 1074-1075: in all other ips calls, when the command fail, you > do raise TAbort(), why not > do that her? Changed
> > DC_tm.py: > > - DC_ips_set_auth() function: why don't you check to make sure > pref_flag and mirr_flag are not > specified at the same time right here? I know you check in > transfer_mod.py too, but then, failing > sooner is better. Sure. Why not. I put a check in. > > - line 207: Nit: It would be less confusing to the reader of the code > to name it "mirror_url" intead > of "pkg_url". So, that would make the statement to be "for mirror_url > in mirror_url_list" Agreed. Done > > - lines 233-236: should we log this regardless quit_on_pkg_failure is > specified or not? Yes. Done. > > - line 240: if the setting of alternate authority failed, that means > we can't set the alternate authority, > why do we need to unset it here? When testing, I found that sometimes it ends up failing late enough that it's still listed. So I put in the unset. > > - line 257-260, 235-356, 377-381, 388: all these should be logged > regardless quit_on_pkg_failure is true or not. Agreed. Done. > > - line 360: again, I don't quite understand why we need to unset the > auth if we can't successfully set them in the first place. > I am sure I misunderstand something, please explain. See above. Jean > > > Jean McCormack wrote: >> Can Karen please review this? Others are welcome to review this if >> they wish. Please let me know as I'd >> like to push this tonight or tomorrow morning. >> >> >> Defects: >> >> 3358 Cleanup repo mirroring items in manifest >> http://defect.opensolaris.org/bz/show_bug.cgi?id=3358 >> >> >> 3360 Allowing specifying the default repo and default authority >> installing pkgs >> http://defect.opensolaris.org/bz/show_bug.cgi?id=3360 >> >> 3668 constructor should use pkg purge-history upon completion >> http://defect.opensolaris.org/bz/show_bug.cgi?id=3668 >> >> >> Webrev: >> >> http://cr.opensolaris.org/~jeanm/DC_pkg_bugs/ >> >> thanks, >> >> Jean >> _______________________________________________ >> caiman-discuss mailing list >> caiman-discuss at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >> >
