Thanks for the reviews. Here's an incremental.
---
python/ovs/unixctl.py | 42 ++++++++++++++++++++++++++++--------------
tests/unixctl-py.at | 4 ++--
2 files changed, 30 insertions(+), 16 deletions(-)
diff --git a/python/ovs/unixctl.py b/python/ovs/unixctl.py
index c36eae2..8921ba8 100644
--- a/python/ovs/unixctl.py
+++ b/python/ovs/unixctl.py
@@ -12,6 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+import copy
import errno
import os
import types
@@ -27,7 +28,7 @@ import ovs.vlog
Message = ovs.jsonrpc.Message
vlog = ovs.vlog.Vlog("unixctl")
commands = {}
-string = types.StringTypes
+strtypes = types.StringTypes
class _UnixctlCommand(object):
@@ -63,8 +64,21 @@ def _unixctl_version(conn, unused_argv, unused_aux):
def command_register(name, usage, min_args, max_args, callback, aux):
- assert isinstance(name, string)
- assert isinstance(usage, string)
+ """ Registers a command with the given 'name' to be exposed by the
+ UnixctlServer. 'usage' describes the arguments to the command; it is used
+ only for presentation to the user in "help" output.
+
+ 'callback' is called when the command is received. It is passed a
+ UnixctlConnection object, the list of arguments as unicode strings, and
+ 'aux'. Normally 'callback' should reply by calling
+ UnixctlConnection.reply() or UnixctlConnection.reply_error() before it
+ returns, but if the command cannot be handled immediately, then it can
+ defer the reply until later. A given connection can only process a single
+ request at a time, so a reply must be made eventually to avoid blocking
+ that connection."""
+
+ assert isinstance(name, strtypes)
+ assert isinstance(usage, strtypes)
assert isinstance(min_args, int)
assert isinstance(max_args, int)
assert isinstance(callback, types.FunctionType)
@@ -75,9 +89,9 @@ def command_register(name, usage, min_args, max_args,
callback, aux):
def socket_name_from_target(target):
- assert isinstance(target, string)
+ assert isinstance(target, strtypes)
- if target[0] == "/":
+ if target.startswith("/"):
return 0, target
pidfile_name = "%s/%s.pid" % (ovs.dirs.RUNDIR, target)
@@ -138,14 +152,14 @@ class UnixctlConnection(object):
def _reply_impl(self, success, body):
assert isinstance(success, bool)
- assert body is None or isinstance(body, string)
+ assert body is None or isinstance(body, strtypes)
assert self._request_id is not None
if body is None:
body = ""
- if len(body) and body[-1] != '\n':
+ if body and not body.endswith("\n"):
body += "\n"
if success:
@@ -176,13 +190,13 @@ class UnixctlConnection(object):
% (method, command.max_args)
else:
for param in params:
- if not isinstance(param, string):
+ if not isinstance(param, strtypes):
error = '"%s" command has non-string argument' % method
break
if error is None:
- str_params = [str(p) for p in params]
- command.callback(self, str_params, command.aux)
+ unicode_params = [unicode(p) for p in params]
+ command.callback(self, unicode_params, command.aux)
if error:
self.reply_error(error)
@@ -207,7 +221,7 @@ class UnixctlServer(object):
vlog.warn("%s: accept failed: %s" % (self._listener.name,
os.strerror(error)))
- for conn in self._conns:
+ for conn in copy.copy(self._conns):
error = conn.run()
if error and error != errno.EAGAIN:
conn._close()
@@ -228,7 +242,7 @@ class UnixctlServer(object):
@staticmethod
def create(path):
- assert path is None or isinstance(path, string)
+ assert path is None or isinstance(path, strtypes)
if path is not None:
path = "punix:%s" % ovs.util.abs_file_name(ovs.dirs.RUNDIR, path)
@@ -254,10 +268,10 @@ class UnixctlClient(object):
self._conn = conn
def transact(self, command, argv):
- assert isinstance(command, string)
+ assert isinstance(command, strtypes)
assert isinstance(argv, list)
for arg in argv:
- assert isinstance(arg, string)
+ assert isinstance(arg, strtypes)
request = Message.create_request(command, argv)
error, reply = self._conn.transact_block(request)
diff --git a/tests/unixctl-py.at b/tests/unixctl-py.at
index 1810c46..1d435ba 100644
--- a/tests/unixctl-py.at
+++ b/tests/unixctl-py.at
@@ -111,14 +111,14 @@ AT_CHECK([$PYTHON $srcdir/appctl.py -t test-unixctl.py
version], [0], [expout])
AT_CHECK([ovs-appctl -t test-unixctl.py echo robot ninja], [0], [stdout])
AT_CHECK([cat stdout], [0], [dnl
-[['robot', 'ninja']]
+[[u'robot', u'ninja']]
])
mv stdout expout
AT_CHECK([$PYTHON $srcdir/appctl.py -t test-unixctl.py echo robot ninja], [0],
[expout])
AT_CHECK([ovs-appctl -t test-unixctl.py echo_error robot ninja], [2], [],
[stderr])
AT_CHECK([cat stderr], [0], [dnl
-[['robot', 'ninja']]
+[[u'robot', u'ninja']]
ovs-appctl: test-unixctl.py: server returned an error
])
sed 's/ovs-appctl/appctl.py/' stderr > experr
--
1.7.9.2
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
