The changes to plan_install look good. brock
jmr wrote: > Thanks Brock: > > I made a few mods here to the install code even though we are not > currently using it to take the tuple and handle the exception_caught > gracefully. > > http://cr.opensolaris.org/~jmr/um_3981__3998_3950_v3_Oct16/ > > JR > > Brock Pytlik wrote: > >> 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 > _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
