I've remerged these changes with the gate, addressed the comments below, and added fixes for issue 3153.
New webrev: http://cr.opensolaris.org/~tmueller/cr-uuid4 Issues: 856 Image properties 1347 optional UUID per image authority 2307 output used before set in testutils.pkgsend 3153 policy settings require-optional, pursue-latest ignored For issue 3153, I changed the references to image.attrs to image.cfg_cache.get_policy. Please see below for responses to the comments. Danek Duvall wrote: > On Mon, Aug 18, 2008 at 03:21:12PM -0500, Tom Mueller (pkg-discuss) wrote: > > >> http://cr.opensolaris.org/~tmueller/cr-856,1347,2307-2/ >> > > client.py: > > - line 1433: if you're creating an authority with this call to > set-authority, and you don't explicitly pass --reset-uuid, a uuid > doesn't get generated, and it doesn't appear to in img.set_authority(), > either. I thought that a uuid would always be attached to an > authority. > Yes. Good catch. I fixed this by changing the image.set_authority method so that when it creates a new authority, it makes sure that the UUID is set. > - line 1565-1570: Try this: > > try: > propname, propvalue = pargs > except ValueError: > usage( ... ) > yes, that's nicer. > - line 1588: you need some message here to indicate what the user did > wrong. > fixed. The message is now: usage( _("pkg: unset-property requires at least one property name")) > - line 1587: just "if not pargs" > fixed. > - line 1590: I think it'd be better either to validate all properties and > error out if one doesn't exist before unsetting them, or to remove all > properties you can, and list the ones that didn't exist at the end. > The latter is probably better, as you can just utilize the KeyError you > get by trying to delete a non-existent property. > Changed it to the latter. Returned 0 if at least one property is unset. > - line 1615, 1619: please find the widest property name instead of > hardcoding 35. > fixed. > - line 1616: "if pargs" > fixed. > pkg.5.txt: > > - line 353: strike "using pkg(1)" > fixed. > filelist.py: > > - line 102: why not just the None object? > The image.uuid attribute is always a string now. This is the value that is sent in the HTTP header. > image.py: > > - line 395: no spaces around equals here. > fixed. > - line 485, 493: won't the dictionary access naturally raise a KeyError? > Yes. I had added these to put the message in. However, since the message cannot really be used for output anyway because of i18n, there really isn't any need for this. Removed. > - line 497: strike "gen_". Perhaps just name the method "properties"? > renamed to properties > - line 1650: why not a normal dictionary fetch, like on line 1049? > The servers list here is either a list of authorities or a list of server URLs that were passed in on the command line. In the latter case, the only element in the dictionary is "origin", so "uuid" will not be there. So we need to deal with the case where "prefix" is not there. However, there is another bug here. If a user does pass a server in on the search command line, then we don't want to send a UUID in that case, but what we do is pass "None" to image.get_uuid which will most likely cause the UUID of the preferred authority to be used (unless there happens to be an authority called "None"). I changed it to check whether the auth has an origin and only get the uuid if it does. > imageconfig.py: > > - line 109: comment is not true -- it'll return the default policy. > fixed. > - line 161: None object? > Per above, UUIDs are always defined to be some string now. > - line 188: this is decoding from utf-8 into the current default > encoding? What happens if that fails, because the default is 7-bit or > some other lame encoding? > > The decode method returns a Unicode string doesn't it? So the default encoding shouldn't matter here. Thanks. Tom _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
