On 7/4/24 16:09, [email protected] wrote:
> From: Jakob Meng <[email protected]>
>
> This patch introduces support for different output formats to Python
> Unixctl* classes and appctl.py, similar to what the previous commit did
> for ovs-appctl.
> In particular, tests/appctl.py gains a global option '-f,--format'
> which allows users to request JSON instead of plain-text for humans.
>
> Reported-at: https://bugzilla.redhat.com/1824861
> Signed-off-by: Jakob Meng <[email protected]>
> ---
> NEWS | 2 ++
> python/ovs/unixctl/__init__.py | 8 ++++++
> python/ovs/unixctl/client.py | 5 ++--
> python/ovs/unixctl/server.py | 52 +++++++++++++++++++++++++++++-----
> tests/appctl.py | 38 ++++++++++++++++++++-----
> tests/unixctl-py.at | 4 +++
> 6 files changed, 93 insertions(+), 16 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index f182647c7..c750ebae2 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -19,6 +19,8 @@ Post-v3.3.0
> per interface 'options:dpdk-lsc-interrupt' to 'false'.
> - Python:
> * Added custom transaction support to the Idl via add_op().
> + * Added support for different output formats like 'json' to Python's
> + unixctl classes.
>
>
> v3.3.0 - 16 Feb 2024
> diff --git a/python/ovs/unixctl/__init__.py b/python/ovs/unixctl/__init__.py
> index 8ee312943..b05f3df72 100644
> --- a/python/ovs/unixctl/__init__.py
> +++ b/python/ovs/unixctl/__init__.py
> @@ -12,6 +12,7 @@
> # See the License for the specific language governing permissions and
> # limitations under the License.
>
> +import enum
> import sys
>
> import ovs.util
> @@ -19,6 +20,13 @@ import ovs.util
> commands = {}
>
>
> [email protected]
> +# FIXME: Use @enum.verify(enum.NAMED_FLAGS) from Python 3.11 when available.
> +class UnixctlOutputFormat(enum.IntFlag):
> + TEXT = 1 << 0
> + JSON = 1 << 1
> +
> +
> class _UnixctlCommand(object):
> def __init__(self, usage, min_args, max_args, callback, aux):
> self.usage = usage
> diff --git a/python/ovs/unixctl/client.py b/python/ovs/unixctl/client.py
> index 8283f99bb..8a6fcb1b9 100644
> --- a/python/ovs/unixctl/client.py
> +++ b/python/ovs/unixctl/client.py
> @@ -14,6 +14,7 @@
>
> import os
>
> +import ovs.json
> import ovs.jsonrpc
> import ovs.stream
> import ovs.util
> @@ -41,10 +42,10 @@ class UnixctlClient(object):
> return error, None, None
>
> if reply.error is not None:
> - return 0, str(reply.error), None
> + return 0, reply.error, None
> else:
> assert reply.result is not None
> - return 0, None, str(reply.result)
> + return 0, None, reply.result
>
> def close(self):
> self._conn.close()
> diff --git a/python/ovs/unixctl/server.py b/python/ovs/unixctl/server.py
> index d24a7092c..855f38d18 100644
> --- a/python/ovs/unixctl/server.py
> +++ b/python/ovs/unixctl/server.py
> @@ -12,6 +12,7 @@
> # See the License for the specific language governing permissions and
> # limitations under the License.
>
> +import argparse
> import copy
> import errno
> import os
> @@ -35,6 +36,7 @@ class UnixctlConnection(object):
> assert isinstance(rpc, ovs.jsonrpc.Connection)
> self._rpc = rpc
> self._request_id = None
> + self._fmt = ovs.unixctl.UnixctlOutputFormat.TEXT
>
> def run(self):
> self._rpc.run()
> @@ -63,10 +65,29 @@ class UnixctlConnection(object):
> return error
>
> def reply(self, body):
> - self._reply_impl(True, body)
> + assert body is None or isinstance(body, str)
> +
> + if body is None:
> + body = ""
> +
> + if self._fmt == ovs.unixctl.UnixctlOutputFormat.JSON:
> + body = {
> + "reply-format": "plain",
> + "reply": body
> + }
> +
> + return self._reply_impl_json(True, body)
> +
> + def reply_json(self, body):
> + self._reply_impl_json(True, body)
>
> def reply_error(self, body):
> - self._reply_impl(False, body)
> + assert body is None or isinstance(body, str)
> +
> + if body is None:
> + body = ""
> +
> + return self._reply_impl_json(False, body)
>
> # Called only by unixctl classes.
> def _close(self):
> @@ -78,15 +99,11 @@ class UnixctlConnection(object):
> if not self._rpc.get_backlog():
> self._rpc.recv_wait(poller)
>
> - def _reply_impl(self, success, body):
> + def _reply_impl_json(self, success, body):
> assert isinstance(success, bool)
> - assert body is None or isinstance(body, str)
>
> assert self._request_id is not None
>
> - if body is None:
> - body = ""
> -
> if success:
> reply = Message.create_reply(body, self._request_id)
> else:
> @@ -133,6 +150,24 @@ def _unixctl_version(conn, unused_argv, version):
> conn.reply(version)
>
>
> +def _unixctl_set_options(conn, argv, unused_aux):
> + assert isinstance(conn, UnixctlConnection)
> +
> + parser = argparse.ArgumentParser()
> + parser.add_argument("--format", default="text",
> + choices=[fmt.name.lower()
> + for fmt in ovs.unixctl.UnixctlOutputFormat])
Should add type=str.lower to have a case-insensitive check
as we have in C code.
> +
> + try:
> + args = parser.parse_args(args=argv)
> + except argparse.ArgumentError as e:
> + conn.reply_error(str(e))
> + return
> +
> + conn._fmt = ovs.unixctl.UnixctlOutputFormat[args.format.upper()]
> + conn.reply(None)
> +
> +
> class UnixctlServer(object):
> def __init__(self, listener):
> assert isinstance(listener, ovs.stream.PassiveStream)
> @@ -207,4 +242,7 @@ class UnixctlServer(object):
> ovs.unixctl.command_register("version", "", 0, 0, _unixctl_version,
> version)
>
> + ovs.unixctl.command_register("set-options", "[--format text|json]",
> 1,
> + 2, _unixctl_set_options, None)
> +
> return 0, UnixctlServer(listener)
> diff --git a/tests/appctl.py b/tests/appctl.py
> index e5cc28138..ce97a5dcf 100644
> --- a/tests/appctl.py
> +++ b/tests/appctl.py
> @@ -37,6 +37,18 @@ def connect_to_target(target):
> return client
>
>
> +def reply_to_string(reply, fmt=ovs.unixctl.UnixctlOutputFormat.TEXT):
> + if fmt == ovs.unixctl.UnixctlOutputFormat.TEXT:
> + body = str(reply)
> + else:
> + body = ovs.json.to_string(reply)
> +
> + if body and not body.endswith("\n"):
> + body += "\n"
> +
> + return body
> +
> +
> def main():
> parser = argparse.ArgumentParser(description="Python Implementation of"
> " ovs-appctl.")
> @@ -49,30 +61,42 @@ 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.unixctl.UnixctlOutputFormat])
Should be case-insensitive.
> args = parser.parse_args()
>
> signal_alarm(int(args.timeout) if args.timeout else None)
>
> ovs.vlog.Vlog.init()
> target = args.target
> + format = ovs.unixctl.UnixctlOutputFormat[args.format.upper()]
> client = connect_to_target(target)
> +
> + if format != ovs.unixctl.UnixctlOutputFormat.TEXT:
> + err_no, error, _ = client.transact(
> + "set-options", ["--format", args.format])
> +
> + if err_no:
> + ovs.util.ovs_fatal(err_no, "%s: transaction error" % target)
> + elif error is not None:
> + sys.stderr.write(reply_to_string(error))
> + ovs.util.ovs_error(0, "%s: server returned an error" % target)
> + sys.exit(2)
> +
> err_no, error, result = client.transact(args.command, args.argv)
> client.close()
>
> if err_no:
> ovs.util.ovs_fatal(err_no, "%s: transaction error" % target)
> elif error is not None:
> - sys.stderr.write(error)
> - if error and not error.endswith("\n"):
> - sys.stderr.write("\n")
> -
> + sys.stderr.write(reply_to_string(error))
> ovs.util.ovs_error(0, "%s: server returned an error" % target)
> sys.exit(2)
> else:
> assert result is not None
> - sys.stdout.write(result)
> - if result and not result.endswith("\n"):
> - sys.stdout.write("\n")
> + sys.stdout.write(reply_to_string(result, format))
>
>
> if __name__ == '__main__':
> diff --git a/tests/unixctl-py.at b/tests/unixctl-py.at
> index 724006118..4d59c5818 100644
> --- a/tests/unixctl-py.at
> +++ b/tests/unixctl-py.at
> @@ -100,6 +100,7 @@ The available commands are:
> exit
> help
> log [[arg ...]]
> + set-options [[--format text|json]]
> version
> vlog/close
> vlog/list
> @@ -112,6 +113,9 @@ AT_CHECK([PYAPPCTL_PY -t test-unixctl.py help], [0],
> [expout])
> AT_CHECK([ovs-vsctl --version | sed 's/ovs-vsctl/test-unixctl.py/' | head -1
> > expout])
> AT_CHECK([APPCTL -t test-unixctl.py version], [0], [expout])
> AT_CHECK([PYAPPCTL_PY -t test-unixctl.py version], [0], [expout])
> +AT_CHECK_UNQUOTED([PYAPPCTL_PY -t test-unixctl.py --format json version],
> [0], [dnl
> +{"reply":"$(cat expout)","reply-format":"plain"}
> +])
>
> AT_CHECK([APPCTL -t test-unixctl.py echo robot ninja], [0], [stdout])
> AT_CHECK([cat stdout | sed -e "s/u'/'/g"], [0], [dnl
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
