| Message ID | 20231025093714.706870-3-jmeng@redhat.com |
|---|---|
| State | RFC, archived |
| Headers | show |
| Series | Add global option to output JSON from ovs-appctl cmds. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On 25 Oct 2023, at 11:37, jmeng@redhat.com wrote: > From: Jakob Meng <code@jakobmeng.de> > > This patch introduces support for different output formats to the > Python code, as did the previous commit for ovs-xxx tools like > 'ovs-appctl --format json dpif/show'. > In particular, tests/appctl.py gains a global option '-f,--format' > which allows users to request JSON instead of plain-text for humans. > > This patch does not yet pass the choosen output format to commands. > Doing so requires changes to all command_register() calls and all > command callbacks. To improve readibility those changes have been > split out into a follow up patch. Respectively, whenever an output > format other than 'text' is choosen for tests/appctl.py, the script > will fail. > > Reported-at: https://bugzilla.redhat.com/1824861 > Signed-off-by: Jakob Meng <code@jakobmeng.de> Note reviewed anything yet, but I get some flake8 errors when compiling your series: pends.py build-aux/soexpand.py build-aux/xml2nroff' && \ flake8 $src --select=H231,H232,H233,H238 && \ flake8 $src --ignore=E121,E123,E125,E126,E127,E128,E129,E131,E203,E722,W503,W504,F811,D,H,I && \ touch flake8-check python/ovs/unixctl/__init__.py:25:50: E261 at least two spaces before inline comment python/ovs/unixctl/__init__.py:30:9: E265 block comment should start with '# ' python/ovs/unixctl/__init__.py:73:68: E261 at least two spaces before inline comment python/ovs/unixctl/server.py:114:27: F821 undefined name 'params' python/ovs/unixctl/server.py:118:19: E111 indentation is not a multiple of 4 python/ovs/unixctl/server.py:148:51: E261 at least two spaces before inline comment python/ovs/unixctl/server.py:239:1: E302 expected 2 blank lines, found 1 make[1]: *** [Makefile:6795: flake8-check] Error 1 FYI if you install flake8 on your system, re-run ./configure it will pick these up as part of your compile process (and make sure you add --enable-Werror). //Eelco > --- > python/ovs/unixctl/__init__.py | 38 +++++++++++++++--- > python/ovs/unixctl/client.py | 20 ++++++++-- > python/ovs/unixctl/server.py | 71 ++++++++++++++++++++++------------ > python/ovs/util.py | 9 +++++ > tests/appctl.py | 7 +++- > 5 files changed, 112 insertions(+), 33 deletions(-) > > diff --git a/python/ovs/unixctl/__init__.py b/python/ovs/unixctl/__init__.py > index 8ee312943..aa8969c61 100644 > --- a/python/ovs/unixctl/__init__.py > +++ b/python/ovs/unixctl/__init__.py > @@ -20,10 +20,14 @@ commands = {} > > > class _UnixctlCommand(object): > - def __init__(self, usage, min_args, max_args, callback, aux): > + # TODO: Output format will be passed as 'fmt' to the command in later > + # patch. > + def __init__(self, usage, min_args, max_args, # fmt, > + callback, aux): > self.usage = usage > self.min_args = min_args > self.max_args = max_args > + #self.fmt = fmt > self.callback = callback > self.aux = aux > > @@ -42,10 +46,12 @@ def _unixctl_help(conn, unused_argv, unused_aux): > conn.reply(reply) > > > -def command_register(name, usage, min_args, max_args, callback, aux): > +def command_register_fmt(name, usage, min_args, max_args, fmt, callback, aux): > """ 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. > + only for presentation to the user in "help" output. 'output_fmts' is a > + bitmap that defines what output formats a command supports, e.g. > + OVS_OUTPUT_FMT_TEXT | OVS_OUTPUT_FMT_JSON. > > 'callback' is called when the command is received. It is passed a > UnixctlConnection object, the list of arguments as unicode strings, and > @@ -60,11 +66,33 @@ def command_register(name, usage, min_args, max_args, callback, aux): > assert isinstance(usage, str) > assert isinstance(min_args, int) > assert isinstance(max_args, int) > + assert isinstance(fmt, ovs.util.OutputFormat) > assert callable(callback) > > if name not in commands: > - commands[name] = _UnixctlCommand(usage, min_args, max_args, callback, > - aux) > + commands[name] = _UnixctlCommand(usage, min_args, max_args, # fmt, > + callback, aux) > + > + > +# TODO: command_register() will be replaced with command_register_fmt() in a > +# later patch of this series. It is is kept temporarily to reduce the > +# amount of changes in this patch. > +def command_register(name, usage, min_args, max_args, callback, aux): > + """ 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.""" > + > + command_register_fmt(name, usage, min_args, max_args, > + ovs.util.OutputFormat.TEXT, callback, aux) > > > def socket_name_from_target(target): > diff --git a/python/ovs/unixctl/client.py b/python/ovs/unixctl/client.py > index 8283f99bb..29a5f3df9 100644 > --- a/python/ovs/unixctl/client.py > +++ b/python/ovs/unixctl/client.py > @@ -26,13 +26,20 @@ class UnixctlClient(object): > assert isinstance(conn, ovs.jsonrpc.Connection) > self._conn = conn > > - def transact(self, command, argv): > + def transact(self, command, argv, fmt): > assert isinstance(command, str) > assert isinstance(argv, list) > for arg in argv: > assert isinstance(arg, str) > + assert isinstance(fmt, ovs.util.OutputFormat) > + plain_rpc = (fmt == ovs.util.OutputFormat.TEXT) > + > + if plain_rpc: > + request = ovs.jsonrpc.Message.create_request(command, argv) > + else: > + request = ovs.jsonrpc.Message.create_request( > + "execute/v1", [command, fmt.name.lower()] + argv) > > - request = ovs.jsonrpc.Message.create_request(command, argv) > error, reply = self._conn.transact_block(request) > > if error: > @@ -41,7 +48,14 @@ class UnixctlClient(object): > return error, None, None > > if reply.error is not None: > - return 0, str(reply.error), None > + plain_rpc_error = ('"%s" is not a valid command' % > + ovs.util.RPC_MARKER) > + if str(reply.error).startswith(plain_rpc_error): > + return 0, "JSON RPC reply indicates incompatible "\ > + "server. Please upgrade server side to "\ > + "newer version.", None > + else: > + return 0, str(reply.error), None > else: > assert reply.result is not None > return 0, None, str(reply.result) > diff --git a/python/ovs/unixctl/server.py b/python/ovs/unixctl/server.py > index 5f9b3e739..7228f8fd9 100644 > --- a/python/ovs/unixctl/server.py > +++ b/python/ovs/unixctl/server.py > @@ -104,30 +104,52 @@ class UnixctlConnection(object): > > self._request_id = request.id > > - error = None > - params = request.params > - method = request.method > - command = ovs.unixctl.commands.get(method) > - if command is None: > - error = '"%s" is not a valid command' % method > - elif len(params) < command.min_args: > - error = '"%s" command requires at least %d arguments' \ > - % (method, command.min_args) > - elif len(params) > command.max_args: > - error = '"%s" command takes at most %d arguments' \ > - % (method, command.max_args) > - else: > - for param in params: > - if not isinstance(param, str): > - error = '"%s" command has non-string argument' % method > - break > + try: > + plain_rpc = request.method != ovs.util.RPC_MARKER > + args_offset = 0 if plain_rpc else 2 > > - if error is None: > - unicode_params = [str(p) for p in params] > - command.callback(self, unicode_params, command.aux) > + if not plain_rpc and len(request.params) < 2: > + raise ValueError( > + "JSON-RPC API mismatch: Unexpected # of params: %d" > + % len(params)) > > - if error: > - self.reply_error(error) > + for param in request.params: > + if not isinstance(param, str): > + raise ValueError( > + "command has non-string argument: %s" % param) > + > + method = request.method if plain_rpc else request.params[0] > + if plain_rpc: > + fmt = ovs.util.OutputFormat.TEXT > + else: > + fmt = ovs.util.OutputFormat[request.params[1].upper()] > + params = request.params[args_offset:] > + > + command = ovs.unixctl.commands.get(method) > + if command is None: > + raise ValueError('"%s" is not a valid command' % method) > + elif len(params) < command.min_args: > + raise ValueError( > + '"%s" command requires at least %d arguments' > + % (method, command.min_args)) > + elif len(params) > command.max_args: > + raise ValueError( > + '"%s" command takes at most %d arguments' > + % (method, command.max_args)) > + > + # TODO: Remove this check once output format will be passed to the > + # command handler below. > + if fmt != ovs.util.OutputFormat.TEXT: > + raise ValueError( > + 'output format "%s" has not been implemented yet' > + % fmt.name.lower()) > + > + unicode_params = [str(p) for p in params] > + command.callback(self, unicode_params, # fmt, > + command.aux) > + > + except Exception as e: > + self.reply_error(str(e)) > > > def _unixctl_version(conn, unused_argv, version): > @@ -207,12 +229,13 @@ class UnixctlServer(object): > % path) > return error, None > > - ovs.unixctl.command_register("version", "", 0, 0, _unixctl_version, > + ovs.unixctl.command_register("version", "", 0, 0, > + _unixctl_version, > version) > > return 0, UnixctlServer(listener) > > - > +# TODO: What is this? A copy of UnixctlClient from client.py? > class UnixctlClient(object): > def __init__(self, conn): > assert isinstance(conn, ovs.jsonrpc.Connection) > diff --git a/python/ovs/util.py b/python/ovs/util.py > index 3dba022f8..6206f90d3 100644 > --- a/python/ovs/util.py > +++ b/python/ovs/util.py > @@ -15,9 +15,18 @@ > import os > import os.path > import sys > +import enum > > PROGRAM_NAME = os.path.basename(sys.argv[0]) > EOF = -1 > +RPC_MARKER = "execute/v1" > + > + > +@enum.unique > +@enum.verify(enum.NAMED_FLAGS) > +class OutputFormat(enum.IntFlag): > + TEXT = 1 << 0 > + JSON = 1 << 1 > > > def abs_file_name(dir_, file_name): > diff --git a/tests/appctl.py b/tests/appctl.py > index b85b364fa..42f96b2a3 100644 > --- a/tests/appctl.py > +++ b/tests/appctl.py > @@ -49,6 +49,10 @@ def main(): > help="Arguments to the command.") > parser.add_argument("-T", "--timeout", metavar="SECS", > help="wait at most SECS seconds for a response") > + parser.add_argument("-f", "--format", metavar="FMT", > + help="Output format.", default="text", > + choices=[fmt.name.lower() > + for fmt in ovs.util.OutputFormat]) > args = parser.parse_args() > > signal_alarm(int(args.timeout) if args.timeout else None) > @@ -56,7 +60,8 @@ def main(): > ovs.vlog.Vlog.init() > target = args.target > client = connect_to_target(target) > - err_no, error, result = client.transact(args.command, args.argv) > + err_no, error, result = client.transact( > + args.command, args.argv, ovs.util.OutputFormat[args.format.upper()]) > client.close() > > if err_no: > -- > 2.39.2 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On 27.10.23 15:52, Eelco Chaudron wrote: > On 25 Oct 2023, at 11:37, jmeng@redhat.com wrote: >> From: Jakob Meng <code@jakobmeng.de> >> >> This patch introduces support for different output formats to the >> Python code, as did the previous commit for ovs-xxx tools like >> 'ovs-appctl --format json dpif/show'. >> In particular, tests/appctl.py gains a global option '-f,--format' >> which allows users to request JSON instead of plain-text for humans. >> >> This patch does not yet pass the choosen output format to commands. >> Doing so requires changes to all command_register() calls and all >> command callbacks. To improve readibility those changes have been >> split out into a follow up patch. Respectively, whenever an output >> format other than 'text' is choosen for tests/appctl.py, the script >> will fail. >> >> Reported-at: https://bugzilla.redhat.com/1824861 >> Signed-off-by: Jakob Meng <code@jakobmeng.de> > Note reviewed anything yet, but I get some flake8 errors when compiling your series: > > pends.py build-aux/soexpand.py build-aux/xml2nroff' && \ > flake8 $src --select=H231,H232,H233,H238 && \ > flake8 $src --ignore=E121,E123,E125,E126,E127,E128,E129,E131,E203,E722,W503,W504,F811,D,H,I && \ > touch flake8-check > python/ovs/unixctl/__init__.py:25:50: E261 at least two spaces before inline comment > python/ovs/unixctl/__init__.py:30:9: E265 block comment should start with '# ' > python/ovs/unixctl/__init__.py:73:68: E261 at least two spaces before inline comment > python/ovs/unixctl/server.py:114:27: F821 undefined name 'params' > python/ovs/unixctl/server.py:118:19: E111 indentation is not a multiple of 4 > python/ovs/unixctl/server.py:148:51: E261 at least two spaces before inline comment > python/ovs/unixctl/server.py:239:1: E302 expected 2 blank lines, found 1 > make[1]: *** [Makefile:6795: flake8-check] Error 1 > > > FYI if you install flake8 on your system, re-run ./configure it will pick these up as part of your compile process (and make sure you add --enable-Werror). > Thank you, Eelco, for pointing this out. I was not aware that flake8 is actually incorporated into the build if available. This reminds me of something which bothers me about the OVS docs: It documents requirements and steps for building and installing OVS in great detail (e.g. [0]). It lists all kinds of configure flags one could use and software packages one could install and often gives rationale for it. But for starters this is overwhelming! Please gimme a brief list of commands to get started on common OSes such as Debian/Ubuntu and/or Fedora/RHEL. For example, put it at the beginning of [0]. A list which reflects what you and other experienced OVS devs typically use in their day to day work. Without experience in OVS, how else are starters supposed to know which configure flags to use and which software to install? [0] https://docs.openvswitch.org/en/latest/intro/install/general/ The benefit for you will hopefully be better patches, without basic shortcomings such as flake8 failures. Wdyt?
On 30 Oct 2023, at 11:50, Jakob Meng wrote: > On 27.10.23 15:52, Eelco Chaudron wrote: >> On 25 Oct 2023, at 11:37, jmeng@redhat.com wrote: >>> From: Jakob Meng <code@jakobmeng.de> >>> >>> This patch introduces support for different output formats to the >>> Python code, as did the previous commit for ovs-xxx tools like >>> 'ovs-appctl --format json dpif/show'. >>> In particular, tests/appctl.py gains a global option '-f,--format' >>> which allows users to request JSON instead of plain-text for humans. >>> >>> This patch does not yet pass the choosen output format to commands. >>> Doing so requires changes to all command_register() calls and all >>> command callbacks. To improve readibility those changes have been >>> split out into a follow up patch. Respectively, whenever an output >>> format other than 'text' is choosen for tests/appctl.py, the script >>> will fail. >>> >>> Reported-at: https://bugzilla.redhat.com/1824861 >>> Signed-off-by: Jakob Meng <code@jakobmeng.de> >> Note reviewed anything yet, but I get some flake8 errors when compiling your series: >> >> pends.py build-aux/soexpand.py build-aux/xml2nroff' && \ >> flake8 $src --select=H231,H232,H233,H238 && \ >> flake8 $src --ignore=E121,E123,E125,E126,E127,E128,E129,E131,E203,E722,W503,W504,F811,D,H,I && \ >> touch flake8-check >> python/ovs/unixctl/__init__.py:25:50: E261 at least two spaces before inline comment >> python/ovs/unixctl/__init__.py:30:9: E265 block comment should start with '# ' >> python/ovs/unixctl/__init__.py:73:68: E261 at least two spaces before inline comment >> python/ovs/unixctl/server.py:114:27: F821 undefined name 'params' >> python/ovs/unixctl/server.py:118:19: E111 indentation is not a multiple of 4 >> python/ovs/unixctl/server.py:148:51: E261 at least two spaces before inline comment >> python/ovs/unixctl/server.py:239:1: E302 expected 2 blank lines, found 1 >> make[1]: *** [Makefile:6795: flake8-check] Error 1 >> >> >> FYI if you install flake8 on your system, re-run ./configure it will pick these up as part of your compile process (and make sure you add --enable-Werror). >> > > Thank you, Eelco, for pointing this out. I was not aware that flake8 is actually incorporated into the build if available. This reminds me of something which bothers me about the OVS docs: > > It documents requirements and steps for building and installing OVS in great detail (e.g. [0]). It lists all kinds of configure flags one could use and software packages one could install and often gives rationale for it. But for starters this is overwhelming! > > Please gimme a brief list of commands to get started on common OSes such as Debian/Ubuntu and/or Fedora/RHEL. For example, put it at the beginning of [0]. A list which reflects what you and other experienced OVS devs typically use in their day to day work. Without experience in OVS, how else are starters supposed to know which configure flags to use and which software to install? I believe all the information you're looking for is actually included in the document you mentioned. I understand that it might appear a bit scattered, but the main issue here is that not everyone takes the time to thoroughly read through it. It's worth noting that all the essential development tools are listed right here: "If you're planning to make significant modifications to Open vSwitch, it's a good idea to install the following tools for better warnings." However, if you have time to submit a patch making the documentation more new user-friendly, that would be fantastic! Unfortunately, it seems that many people tend to overlook the finer details. For instance, very few actually follow through with running utilities/checkpatch.py, as suggested in the submitting-patches section. 😊 The following gives an idea of what the CI installs for Ubuntu; https://github.com/chaudron/ovs/tree/master/.ci or if you want my personal Fedora set (does not include sparse): https://github.com/chaudron/ovs_dp_test/blob/main/Vagrantfile Cheers, Eelco > [0] https://docs.openvswitch.org/en/latest/intro/install/general/ > > The benefit for you will hopefully be better patches, without basic shortcomings such as flake8 failures. > > Wdyt?
diff --git a/python/ovs/unixctl/__init__.py b/python/ovs/unixctl/__init__.py index 8ee312943..aa8969c61 100644 --- a/python/ovs/unixctl/__init__.py +++ b/python/ovs/unixctl/__init__.py @@ -20,10 +20,14 @@ commands = {} class _UnixctlCommand(object): - def __init__(self, usage, min_args, max_args, callback, aux): + # TODO: Output format will be passed as 'fmt' to the command in later + # patch. + def __init__(self, usage, min_args, max_args, # fmt, + callback, aux): self.usage = usage self.min_args = min_args self.max_args = max_args + #self.fmt = fmt self.callback = callback self.aux = aux @@ -42,10 +46,12 @@ def _unixctl_help(conn, unused_argv, unused_aux): conn.reply(reply) -def command_register(name, usage, min_args, max_args, callback, aux): +def command_register_fmt(name, usage, min_args, max_args, fmt, callback, aux): """ 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. + only for presentation to the user in "help" output. 'output_fmts' is a + bitmap that defines what output formats a command supports, e.g. + OVS_OUTPUT_FMT_TEXT | OVS_OUTPUT_FMT_JSON. 'callback' is called when the command is received. It is passed a UnixctlConnection object, the list of arguments as unicode strings, and @@ -60,11 +66,33 @@ def command_register(name, usage, min_args, max_args, callback, aux): assert isinstance(usage, str) assert isinstance(min_args, int) assert isinstance(max_args, int) + assert isinstance(fmt, ovs.util.OutputFormat) assert callable(callback) if name not in commands: - commands[name] = _UnixctlCommand(usage, min_args, max_args, callback, - aux) + commands[name] = _UnixctlCommand(usage, min_args, max_args, # fmt, + callback, aux) + + +# TODO: command_register() will be replaced with command_register_fmt() in a +# later patch of this series. It is is kept temporarily to reduce the +# amount of changes in this patch. +def command_register(name, usage, min_args, max_args, callback, aux): + """ 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.""" + + command_register_fmt(name, usage, min_args, max_args, + ovs.util.OutputFormat.TEXT, callback, aux) def socket_name_from_target(target): diff --git a/python/ovs/unixctl/client.py b/python/ovs/unixctl/client.py index 8283f99bb..29a5f3df9 100644 --- a/python/ovs/unixctl/client.py +++ b/python/ovs/unixctl/client.py @@ -26,13 +26,20 @@ class UnixctlClient(object): assert isinstance(conn, ovs.jsonrpc.Connection) self._conn = conn - def transact(self, command, argv): + def transact(self, command, argv, fmt): assert isinstance(command, str) assert isinstance(argv, list) for arg in argv: assert isinstance(arg, str) + assert isinstance(fmt, ovs.util.OutputFormat) + plain_rpc = (fmt == ovs.util.OutputFormat.TEXT) + + if plain_rpc: + request = ovs.jsonrpc.Message.create_request(command, argv) + else: + request = ovs.jsonrpc.Message.create_request( + "execute/v1", [command, fmt.name.lower()] + argv) - request = ovs.jsonrpc.Message.create_request(command, argv) error, reply = self._conn.transact_block(request) if error: @@ -41,7 +48,14 @@ class UnixctlClient(object): return error, None, None if reply.error is not None: - return 0, str(reply.error), None + plain_rpc_error = ('"%s" is not a valid command' % + ovs.util.RPC_MARKER) + if str(reply.error).startswith(plain_rpc_error): + return 0, "JSON RPC reply indicates incompatible "\ + "server. Please upgrade server side to "\ + "newer version.", None + else: + return 0, str(reply.error), None else: assert reply.result is not None return 0, None, str(reply.result) diff --git a/python/ovs/unixctl/server.py b/python/ovs/unixctl/server.py index 5f9b3e739..7228f8fd9 100644 --- a/python/ovs/unixctl/server.py +++ b/python/ovs/unixctl/server.py @@ -104,30 +104,52 @@ class UnixctlConnection(object): self._request_id = request.id - error = None - params = request.params - method = request.method - command = ovs.unixctl.commands.get(method) - if command is None: - error = '"%s" is not a valid command' % method - elif len(params) < command.min_args: - error = '"%s" command requires at least %d arguments' \ - % (method, command.min_args) - elif len(params) > command.max_args: - error = '"%s" command takes at most %d arguments' \ - % (method, command.max_args) - else: - for param in params: - if not isinstance(param, str): - error = '"%s" command has non-string argument' % method - break + try: + plain_rpc = request.method != ovs.util.RPC_MARKER + args_offset = 0 if plain_rpc else 2 - if error is None: - unicode_params = [str(p) for p in params] - command.callback(self, unicode_params, command.aux) + if not plain_rpc and len(request.params) < 2: + raise ValueError( + "JSON-RPC API mismatch: Unexpected # of params: %d" + % len(params)) - if error: - self.reply_error(error) + for param in request.params: + if not isinstance(param, str): + raise ValueError( + "command has non-string argument: %s" % param) + + method = request.method if plain_rpc else request.params[0] + if plain_rpc: + fmt = ovs.util.OutputFormat.TEXT + else: + fmt = ovs.util.OutputFormat[request.params[1].upper()] + params = request.params[args_offset:] + + command = ovs.unixctl.commands.get(method) + if command is None: + raise ValueError('"%s" is not a valid command' % method) + elif len(params) < command.min_args: + raise ValueError( + '"%s" command requires at least %d arguments' + % (method, command.min_args)) + elif len(params) > command.max_args: + raise ValueError( + '"%s" command takes at most %d arguments' + % (method, command.max_args)) + + # TODO: Remove this check once output format will be passed to the + # command handler below. + if fmt != ovs.util.OutputFormat.TEXT: + raise ValueError( + 'output format "%s" has not been implemented yet' + % fmt.name.lower()) + + unicode_params = [str(p) for p in params] + command.callback(self, unicode_params, # fmt, + command.aux) + + except Exception as e: + self.reply_error(str(e)) def _unixctl_version(conn, unused_argv, version): @@ -207,12 +229,13 @@ class UnixctlServer(object): % path) return error, None - ovs.unixctl.command_register("version", "", 0, 0, _unixctl_version, + ovs.unixctl.command_register("version", "", 0, 0, + _unixctl_version, version) return 0, UnixctlServer(listener) - +# TODO: What is this? A copy of UnixctlClient from client.py? class UnixctlClient(object): def __init__(self, conn): assert isinstance(conn, ovs.jsonrpc.Connection) diff --git a/python/ovs/util.py b/python/ovs/util.py index 3dba022f8..6206f90d3 100644 --- a/python/ovs/util.py +++ b/python/ovs/util.py @@ -15,9 +15,18 @@ import os import os.path import sys +import enum PROGRAM_NAME = os.path.basename(sys.argv[0]) EOF = -1 +RPC_MARKER = "execute/v1" + + +@enum.unique +@enum.verify(enum.NAMED_FLAGS) +class OutputFormat(enum.IntFlag): + TEXT = 1 << 0 + JSON = 1 << 1 def abs_file_name(dir_, file_name): diff --git a/tests/appctl.py b/tests/appctl.py index b85b364fa..42f96b2a3 100644 --- a/tests/appctl.py +++ b/tests/appctl.py @@ -49,6 +49,10 @@ def main(): help="Arguments to the command.") parser.add_argument("-T", "--timeout", metavar="SECS", help="wait at most SECS seconds for a response") + parser.add_argument("-f", "--format", metavar="FMT", + help="Output format.", default="text", + choices=[fmt.name.lower() + for fmt in ovs.util.OutputFormat]) args = parser.parse_args() signal_alarm(int(args.timeout) if args.timeout else None) @@ -56,7 +60,8 @@ def main(): ovs.vlog.Vlog.init() target = args.target client = connect_to_target(target) - err_no, error, result = client.transact(args.command, args.argv) + err_no, error, result = client.transact( + args.command, args.argv, ovs.util.OutputFormat[args.format.upper()]) client.close() if err_no: