Please see inline.
Shawn Walker wrote:
2008/6/24 Tom Mueller (pkg-discuss) <[EMAIL PROTECTED]>:
Please review the changes for the following issues:
1347 optional UUID per image
2307 output used before set in testutils.pkgsend
Here is the webrev: http://cr.opensolaris.org/~tmueller/cr-1347/
http://cr.opensolaris.org/~tmueller/cr-1347/src/client.py.wdiff.html
==========
1154 + _("pkg: set-authority: one one of
--reset-uuid or " \
s/one one of/only one of/ ?
Fixed.
1187 + uuid = None
1188 + if reset_uuid:
1189 + uuid = pkg.Uuid25.uuid1()
1190 + if unset_uuid:
1191 + uuid = str(None)
Could you drop 1190-1191 and just change 1187 to uuid = str(None)?
Would that be equivalent here?
No. uuid = None means leave the uuid what it currently is. uuid =
"None", i.e., str(None), means reset the UUID to the value "None". Maybe
line 1191 should be uuid = "None" instead.
217 + this image to the authority. With --unset-uuid,
return the unique
Two spaces before "With" to be consistent with the rest of the text.
Fixed.
http://cr.opensolaris.org/~tmueller/cr-1347/src/modules/client/image.py.wdiff.html
==========
319 + def get_uuid(self, authority = None):
320 + """ Return the UUID for the specified
authority prefix. If the
321 + authority isn't specified, use the
preferred authority.
322 + """
323 + if authority is None:
324 + authority =
self.cfg_cache.preferred_authority
325 + try:
326 + authent =
self.cfg_cache.authorities[authority]
327 + except KeyError:
328 + authority =
self.cfg_cache.preferred_authority
329 + authent =
self.cfg_cache.authorities[authority]
Shouldn't this be more like:
def get_uuid(self, authority = None):
""" Return the UUID for the specified authority prefix. If the
authority isn't specified, use the preferred authority.
"""
if authority is None or not self.has_authority(authority):
authority = self.get_default_authority()
authent = self.get_authority(authority)
return authent["uuid"]
?
That does look better. I actually copied the structure from the
preceding get_ssl_credentials method. That one could be rewritten too.
http://cr.opensolaris.org/~tmueller/cr-1347/src/modules/misc.py.wdiff.html
==========
59 59 def versioned_urlopen(base_uri, operation, versions = [],
tail = None,
60 - data = None, headers = {}, ssl_creds = None, imgtype = IMG_NONE):
60 + data = None, headers = {}, ssl_creds = None, imgtype = IMG_NONE,
61 + uuid = "None"):
We're quickly going to end up with a lot of parameters here if we have
to explicitly name each item.
Would it be better to just use the headers parameter we already have for this?
With the way it is now, I was able to change from having the UUID in the
User-agent to having it in a header by just changing a couple of lines
in the version_urlopen method. If we expect every caller of
version_urlopen to fill in headers, we wouldn't be able to do that.
It might make sense to just pass in an image to version_urlopen so that
it could just pull everything that it wants out of the image.
This change adds the file Uuid25.py to the pkg module. This is a copy
of the file from python 2.5 and it has been brought in in the same way
as Queue25.py, i.e, uppercased the module name, did nothing with any
headers in the file. That's why there isn't any Sun copyright in that
file.
I would think we still want / need an indicator somewhere of where the
code came from and what license it is under.
I didn't see anything like this for Queue25.py. Does anyone know what
the header should be?
My final comment is that the original bug says "UUID per image" -- but
this actually looks like UUID per authority per image. In fact, the
original bug never mentions authorities.
I'm assuming that was intentional?
Yes, per the design discussion that was held on pkg-discuss about this.
I changed the summary on the issue and when I commit this again, I'll
get the new summary in the changelog.
Thanks.
Tom
begin:vcard
fn:Tom Mueller
n:Mueller;Tom
org:Sun Microsystems, Inc.;Update Center Software
adr:;;21915 Hillandale Dr;Elkhorn;NE;68022;USA
email;internet:[EMAIL PROTECTED]
title:Senior Staff Engineer
tel;work:877-250-4011
tel;fax:877-250-4011
tel;home:402-916-9943
x-mozilla-html:TRUE
version:2.1
end:vcard
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
