LGTM. Brock
jmr wrote: > Brock, thanks for taking a look. Comments below: > > JR > > Brock Pytlik wrote: > >> updatemanager.py >> Line 1129: Please don't just bump the version without making sure the >> code works with the bumped version. The reason we have API versioning >> is so that we fail early if the API is changed incompatibly rather >> than waiting till we happen to travel down the changed code path. In >> this case, plan_install now returns a tuple, not a boolean. As far as >> I can tell, this line hasn't been updated to accept the new return value. >> > We only have Update All exposed and are not showing Install Updates to > the user, so this was not tested. I did bump the version and test the > Update All functionality which worked fine. I will make the change here > and test the Install Update using a debug mode switch to turn this back on. > > What should I do on partial success of plan_install? Is it safe to > continue? Is it just a refresh that could have failed? We are not doing > a catalog refresh when the UM is launched from the notification panel, > as one has already been done externally for the notification to be raised. > If you're not refreshing the catalog there, then there's no sense of partial success. The only way, currently, to get into an ambiguous state is if catalog refresh failed for one or more authorities. >> I'm also unclear as to the purpose of __get_api_obj. If the api object >> isn't created on first trial, why would it succeed after that? >> > It would not. I wrapped this as we can fetch the api object in a few > places now, depending on the code path. Previously I had done the api > fetch up front while getting the updates list, but then when I failed > with the version exception I could not output the error messages to the > details pane. Hence the change. > > Ok >> I'll give more feedback in a bit when I get back from a meeting. >> >> Brock >> >> jmr wrote: >> >>> Hi, >>> >>> Simple patch: >>> http://cr.opensolaris.org/~jmr/um_3981__3998_3950_v2_Oct16 >>> >>> Bumping API version to 1. UM currently failing in the gate as the >>> underlying API has bumped the version and the call to: >>> >>> api.ImageInterface(...CLIENT_API_VERSION...) with version 0 >>> >>> Throws a version mismatch exception. Also changed how this error was >>> reported in the Details pane. >>> http://defect.opensolaris.org/bz/show_bug.cgi?id=3998 >>> >>> Range of minor UI changes to conform to UI Spec. >>> http://defect.opensolaris.org/bz/show_bug.cgi?id=3981 >>> >>> Change Copyright file to short version: >>> http://defect.opensolaris.org/bz/show_bug.cgi?id=3950 >>> And removed SUNWipkg-um.import - David Comay told us it's no longer >>> needed. >>> >>> Thanks, >>> >>> JR >>> _______________________________________________ >>> pkg-discuss mailing list >>> [email protected] >>> http://mail.opensolaris.org/mailman/listinfo/pkg-discuss >>> >>> > > _______________________________________________ > pkg-discuss mailing list > [email protected] > http://mail.opensolaris.org/mailman/listinfo/pkg-discuss > _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
