Dan Price wrote: > On Fri 26 Sep 2008 at 03:42PM, Brock Pytlik wrote: > >> Shawn Walker wrote: >> >>> Brock Pytlik wrote: >>> >>>> Webrev: >>>> http://cr.opensolaris.org/~bpytlik/ips-3618-v1/src/packagemanager.py.wdiff.html >>>> >>>> >>>> >>>> Bug: >>>> Manifest retrieval error needs more info and to not show a traceback >>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=3618 >>>> >>>> While I'd like to fix the source of the underlying bug (1133), I feel >>>> like we have little way of telling what the actual problem. Since the >>>> problem is hard to reproduce, I'm suggesting we try this approach to >>>> allow us to gather more information when problems are encountered. It >>>> also removes the traceback from what the user is shown. >>>> >>> I think you meant: >>> http://cr.opensolaris.org/~bpytlik/ips-3618-v1/ >>> >>> client.py: >>> lines 2189, 2190: wording suggestion: "An error was encountered >>> while attempting to retrieve package or file data for the requested >>> operation." >>> >>> retrieve.py: >>> lines 35,42: s/Retrival/Retrieval/ fixing this here means changing >>> it everywhere else, of course >>> >>> lines 36, 42: docstrings are appreciated >>> >>> Otherwise, looks fine. >>> >>> installupdate.py: >>> line 364: I know this isn't your fault, but can you: >>> s/Attemping/Attempting/ >>> >>> Cheers, >>> >> New webrev reflecting shawn's comments at: >> http://cr.opensolaris.org/~bpytlik/ips-3618-v2/ >> > > I've reviewed this. It looks good. Just cleanup one nit > (I don't need to see the review again). > > In client.py, the other except stanzas are for some reason > using '__e' for some reason. Could you either fix them to > be consistent with your thing, or vice versa? I assume the > idea here is to avoid introducing variables into the global > namespace, since we're not in any function here > (although it seems to me that it would have been easier to > just create a function...). > > I presume that pull.py is suffiently different that it doesn't > need to change to use this new exception? > I hadn't considered pull.py in my first pass. I'm a little unclear as to how pull.py/pkgrecv fits into the rest of our structure. Given the structure of the rest of its code, and its largely independent nature, I'm inclined to leave it alone for now. Also, I mostly put these exceptions in to improve the information we're getting when things blow up. I haven't seen any reports from pkgrecv about manifest retrieval failing.
Sound reasonable? Brock > -dp > > _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
