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 here? 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. - 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" - lines 233-236: should we log this regardless quit_on_pkg_failure is specified or not? - 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? - line 257-260, 235-356, 377-381, 388: all these should be logged regardless quit_on_pkg_failure is true or not. - 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. 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 >
