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/ ?

     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?

      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.

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"]
?

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?

> 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.

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?

-- 
Shawn Walker
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to