[ovs-dev] [python idl 12/16] ovs.ovsuuid: Get rid of ovs.ovsuuid.UUID class.
Ethan Jackson
ethan at nicira.com
Wed Sep 21 00:04:32 UTC 2011
> 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.
Perhaps we should write it this way, catch the exception, log our more
informative error message, and re-raise? Is that generally more
pythonish?
Ethan
>
>>
>> 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
>
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
>
More information about the dev
mailing list