> I noticed that there's some code that insists on "string" objects in
> various places, which makes me a little nervous because "string" and
> "unicode" objects in Python seem almost interchangeable but they are
> not the same and a test for one will not accept the other. There's a
> lot of "if type(json) in [str, unicode]" type code in the OVS python
> directory for that reason.
"string" in the unixctl python directory is defined as
types.StringTypes which is specified as follows:
"A sequence containing StringType and UnicodeType used to facilitate
easier checking for any string object. Using this is more portable
than using a sequence of the two string types constructed elsewhere
since it only contains UnicodeType if it has been built in the running
version of Python. For example: isinstance(s, types.StringTypes)."
> I keep hearing that string concatenation in Python is really slow.
> How about this (untested):
Yah it certainly is slower, but I think it's more readable. This
feels like premature optimization to me. I don't feel strongly about
it though. If you think it's important I'll change it.
> ""[0] yields IndexError. Would target.startswith("/") be safer?
Good Idea, I changed it.
>
> In UnixctlConnection._reply_impl(), I think that "if not
> body.endswith('\n')" would be more straightforward:
>> + if len(body) and body[-1] != '\n':
>> + body += "\n"
Changed
>
> I don't quite understand the code below, in
> UnixctlConnection._process_command(). First we check that all of the
> arguments are "string" objects, then we convert them to strings with
> "str"?
So in the first loop we're checking that they are either str or
unicode, and in the second loop, we convert them to str if they happen
to be unicode. I struggled with whether or not this is the correct
thing to do as I wrote this. Since unixctl commands are sort of
supposed to feel like a command line argc, argv main function, it
seems more correct to pass in strings than unicodes. But one could
argue that this isn't correct too I suppose. Do you feel strongly
about it? I'm not really sure what's correct.
> Is it OK to remove elements from a list you're iterating? (I doubt
> it.)
>> + for conn in self._conns:
>> + error = conn.run()
>> + if error and error != errno.EAGAIN:
>> + conn._close()
>> + self._conns.remove(conn)
I tried it out, and it looks like it doesn't work. Good catch!
I'll send out an incremental after I receive feedback to the above comments.
Ethan
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
