[ovs-dev] [python idl 12/16] ovs.ovsuuid: Get rid of ovs.ovsuuid.UUID class.
Reid Price
reid at nicira.com
Wed Sep 21 01:15:36 UTC 2011
On Tue, Sep 20, 2011 at 5:04 PM, Ethan Jackson <ethan at nicira.com> wrote:
> > 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?
>
I would say it is, though it obviously could be taken to an unhealthy
extreme. I think what you suggested is healthy, but there would not be much
in the way of tangible gains until similar functionality is added, or the
validity checking functions are modified.
-Reid
> 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
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20110920/1b7216c0/attachment-0003.html>
More information about the dev
mailing list