[ovs-dev] [python idl 12/16] ovs.ovsuuid: Get rid of ovs.ovsuuid.UUID class.
Reid Price
reid at nicira.com
Tue Sep 20 23:55:36 UTC 2011
Some notes inline. One typo.
On Mon, Sep 19, 2011 at 11:18 AM, Ben Pfaff <blp at nicira.com> wrote:
> This class only caused unnecessary confusion. This commit changes all of
> its methods into top-level functions.
> ---
> python/ovs/db/data.py | 6 ++--
> python/ovs/db/idl.py | 4 +-
> python/ovs/db/types.py | 2 +-
> python/ovs/ovsuuid.py | 80
> +++++++++++++++++++++--------------------------
> tests/test-ovsdb.py | 2 +-
> 5 files changed, 43 insertions(+), 51 deletions(-)
>
> diff --git a/python/ovs/db/data.py b/python/ovs/db/data.py
> index c07d8bf..15a7151 100644
> --- a/python/ovs/db/data.py
> +++ b/python/ovs/db/data.py
> @@ -96,7 +96,7 @@ class Atom(object):
> or (type_ == ovs.db.types.StringType and type(json) in [str,
> unicode])):
> atom = Atom(type_, json)
> elif type_ == ovs.db.types.UuidType:
> - atom = Atom(type_, ovs.ovsuuid.UUID.from_json(json, symtab))
> + atom = Atom(type_, ovs.ovsuuid.from_json(json, symtab))
> else:
> raise error.Error("expected %s" % type_.to_string(), json)
> atom.check_constraints(base)
> @@ -147,7 +147,7 @@ class Atom(object):
>
> def to_json(self):
> if self.type == ovs.db.types.UuidType:
> - return self.value.to_json()
> + return ovs.ovsuuid.to_json(self.value)
> else:
> return self.value
>
> @@ -165,7 +165,7 @@ class Atom(object):
> return ['%s.string = xstrdup("%s");'
> % (var, escapeCString(self.value))]
> elif self.type == ovs.db.types.UuidType:
> - return self.value.cInitUUID(var)
> + return ovs.ovsuuid.to_c_assignment(self.value, var)
>
> def toEnglish(self, escapeLiteral=returnUnchanged):
> if self.type == ovs.db.types.IntegerType:
> diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
> index 65f6838..7841a89 100644
> --- a/python/ovs/db/idl.py
> +++ b/python/ovs/db/idl.py
> @@ -194,12 +194,12 @@ class Idl:
> 'an object' % table_name, table_update)
>
> for uuid_string, row_update in table_update.iteritems():
> - if not ovs.ovsuuid.UUID.is_valid_string(uuid_string):
> + if not ovs.ovsuuid.is_valid_string(uuid_string):
> raise error.Error('<table-update> for table "%s" '
> 'contains bad UUID "%s" as member '
> 'name' % (table_name, uuid_string),
> table_update)
> - uuid = ovs.ovsuuid.UUID.from_string(uuid_string)
> + uuid = ovs.ovsuuid.from_string(uuid_string)
>
I'm getting enough deja vu to know I've said this before =/
If it wasn't for your dastardly-ly informative error messages, this would be
better-written as
uuid = ovs.<...>.from_string(uuid_string)
and letting that function do the raising. Fine as-is, of course.
>
> if type(row_update) != dict:
> raise error.Error('<table-update> for table "%s" '
> diff --git a/python/ovs/db/types.py b/python/ovs/db/types.py
> index dc19f85..9563211 100644
> --- a/python/ovs/db/types.py
> +++ b/python/ovs/db/types.py
> @@ -56,7 +56,7 @@ IntegerType = AtomicType("integer", 0)
> RealType = AtomicType("real", 0.0)
> BooleanType = AtomicType("boolean", False)
> StringType = AtomicType("string", "")
> -UuidType = AtomicType("uuid", ovs.ovsuuid.UUID.zero())
> +UuidType = AtomicType("uuid", ovs.ovsuuid.zero())
>
> ATOMIC_TYPES = [VoidType, IntegerType, RealType, BooleanType, StringType,
> UuidType]
> diff --git a/python/ovs/ovsuuid.py b/python/ovs/ovsuuid.py
> index 5d2785c..0776dd5 100644
> --- a/python/ovs/ovsuuid.py
> +++ b/python/ovs/ovsuuid.py
> @@ -18,57 +18,49 @@ import uuid
> from ovs.db import error
> import ovs.db.parser
>
> -class UUID(uuid.UUID):
> - x = "[0-9a-fA-f]"
> - uuidRE = re.compile("^(%s{8})-(%s{4})-(%s{4})-(%s{4})-(%s{4})(%s{8})$"
> - % (x, x, x, x, x, x))
> +uuidRE = re.compile("^(%s{8})-(%s{4})-(%s{4})-(%s{4})-(%s{4})(%s{8})$"
> + % (("[0-9a-fA-f]",) * 6))
>
Typo, second F should be capitalized. Could also use the dictionary-style
string formatting, though this seems reasonable as-is.
"^(%(hex)s{8})-(%(hex)s{4})- ... " % {'hex': "[0-9a-fA-F]"}
>
> - def __init__(self, s):
> - uuid.UUID.__init__(self, hex=s)
> +def zero():
> + return uuid.UUID(int=0)
>
> - @staticmethod
> - def zero():
> - return UUID('00000000-0000-0000-0000-000000000000')
> +def is_zero(uuid_):
> + return uuid_.int == 0
>
> - def is_zero(self):
> - return self.int == 0
> +def is_valid_string(s):
> + return uuidRE.match(s) is not None
>
> - @staticmethod
> - def is_valid_string(s):
> - return UUID.uuidRE.match(s) != None
> +def from_string(s):
> + if not is_valid_string(s):
> + raise error.Error("%s is not a valid UUID" % s)
> + return uuid.UUID(s)
>
I have a preference for the OO organizational style [I think u.is_zero()
makes more sense than ovs.ovsuuid.is_zero(u)], but I agree that these
changes are better than having a half-classed version that is mixing and
matching elsewhere. If the long string were the problem, you can always do
"UUID = ovs.ovsuuid.UUID" up at the top.
Wanted to note that in general the errors provided by UUID itself are fairly
informative, though they lack the object, e.g.:
>>> import uuid
>>> u = uuid.uuid4()
>>> u
UUID('7862e9f1-6cd5-4922-9d21-6132b6c2bd23')
>>> str(u)
'7862e9f1-6cd5-4922-9d21-6132b6c2bd23'
>>> uuid.UUID(str(u))
UUID('7862e9f1-6cd5-4922-9d21-6132b6c2bd23')
>>> uuid.UUID(str(u)[:4])
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/lib/python2.6/uuid.py", line 134, in __init__
raise ValueError('badly formed hexadecimal UUID string')
ValueError: badly formed hexadecimal UUID string
>>> uuid.UUID(u)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/lib/python2.6/uuid.py", line 131, in __init__
hex = hex.replace('urn:', '').replace('uuid:', '')
AttributeError: 'UUID' object has no attribute 'replace'
>
> - @staticmethod
> - def from_string(s):
> - if not UUID.is_valid_string(s):
> - raise error.Error("%s is not a valid UUID" % s)
> - return UUID(s)
> -
> - @staticmethod
> - def from_json(json, symtab=None):
> +def from_json(json, symtab=None):
> + try:
> + s = ovs.db.parser.unwrap_json(json, "uuid", unicode)
> + if not uuidRE.match(s):
> + raise error.Error("\"%s\" is not a valid UUID" % s, json)
> + return uuid.UUID(s)
> + except error.Error, e:
> + if not symtab:
> + raise e
> try:
> - s = ovs.db.parser.unwrap_json(json, "uuid", unicode)
> - if not UUID.uuidRE.match(s):
> - raise error.Error("\"%s\" is not a valid UUID" % s, json)
> - return UUID(s)
> - except error.Error, e:
> - if not symtab:
> - raise e
> - try:
> - name = ovs.db.parser.unwrap_json(json, "named-uuid",
> unicode)
> - except error.Error:
> - raise e
> + name = ovs.db.parser.unwrap_json(json, "named-uuid", unicode)
> + except error.Error:
> + raise e
>
> - if name not in symtab:
> - symtab[name] = uuid4()
> - return symtab[name]
> + if name not in symtab:
> + symtab[name] = uuid4()
> + return symtab[name]
>
> - def to_json(self):
> - return ["uuid", str(self)]
> +def to_json(uuid_):
> + return ["uuid", str(uuid_)]
>
> - def cInitUUID(self, var):
> - m = UUID.uuidRE.match(str(self))
> - return ["%s.parts[0] = 0x%s;" % (var, m.group(1)),
> - "%s.parts[1] = 0x%s%s;" % (var, m.group(2), m.group(3)),
> - "%s.parts[2] = 0x%s%s;" % (var, m.group(4), m.group(5)),
> - "%s.parts[3] = 0x%s;" % (var, m.group(6))]
> +def to_c_assignment(uuid_, var):
> + """Returns an array of strings, each of which contain a C statement.
> The
> + statements assign 'uuid_' to a "struct uuid" as defined in Open
> vSwitch
> + lib/uuid.h."""
>
> + hex = uuid_.hex
> + return ["%s.parts[%d] = 0x%s;" % (var, x, hex[x * 8:(x + 1) * 8])
> + for x in range(4)]
>
hex is a (rather convenient) builtin you might want to avoid shadowing, and
might even be able to use here (or just "0x%x"), didn't look beyond patch
context though.
> diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py
> index ea45f9a..1774dd9 100644
> --- a/tests/test-ovsdb.py
> +++ b/tests/test-ovsdb.py
> @@ -163,7 +163,7 @@ def substitute_uuids(json, symtab):
> return json
>
> def parse_uuids(json, symtab):
> - if type(json) in [str, unicode] and
> ovs.ovsuuid.UUID.is_valid_string(json):
> + if type(json) in [str, unicode] and ovs.ovsuuid.is_valid_string(json):
> name = "#%d#" % len(symtab)
> sys.stderr.write("%s = %s\n" % (name, json))
> symtab[name] = json
-Reid
> --
> 1.7.4.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20110920/5e20c5f1/attachment-0003.html>
More information about the dev
mailing list