| Message ID | 20231026121044.712645-1-jmeng@redhat.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [ovs-dev] python: Remove duplicate UnixctlClient implementation. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On Thu, Oct 26, 2023 at 02:10:44PM +0200, jmeng@redhat.com wrote: > From: Jakob Meng <code@jakobmeng.de> > > The unixctl implementation in Python has been split into three parts in > the past. During this process the UnixctlClient was duplicated, in > python/ovs/unixctl/client.py and python/ovs/unixctl/server.py. This > patch removes the duplicate from the latter. > > Fixes: 53cf9963ccc6 ("python: Break unixctl implementation into re...") > Signed-off-by: Jakob Meng <code@jakobmeng.de> Thanks Jakob, I agree that this class is duplicated, and that the duplication was introduced by the cited commit. Acked-by: Simon Horman <horms@ovn.org>
On 26 Oct 2023, at 14:10, jmeng@redhat.com wrote: > From: Jakob Meng <code@jakobmeng.de> > > The unixctl implementation in Python has been split into three parts in > the past. During this process the UnixctlClient was duplicated, in > python/ovs/unixctl/client.py and python/ovs/unixctl/server.py. This > patch removes the duplicate from the latter. > > Fixes: 53cf9963ccc6 ("python: Break unixctl implementation into re...") > Signed-off-by: Jakob Meng <code@jakobmeng.de> Removing this duplicate code seems a reasonable thing to do ;) Acked-by: Eelco Chaudron <echaudro@redhat.com>
On 10/26/23 14:10, jmeng@redhat.com wrote: > From: Jakob Meng <code@jakobmeng.de> > > The unixctl implementation in Python has been split into three parts in > the past. During this process the UnixctlClient was duplicated, in > python/ovs/unixctl/client.py and python/ovs/unixctl/server.py. This > patch removes the duplicate from the latter. > > Fixes: 53cf9963ccc6 ("python: Break unixctl implementation into re...") Please, don't abbreviate tags, i.e. the Fixes tag should contain the full commit name regardless of the length. Otherwise, the change seem fine. > Signed-off-by: Jakob Meng <code@jakobmeng.de> > --- > python/ovs/unixctl/server.py | 44 ------------------------------------ > 1 file changed, 44 deletions(-)
On 28.10.23 00:08, Ilya Maximets wrote: > On 10/26/23 14:10, jmeng@redhat.com wrote: >> From: Jakob Meng <code@jakobmeng.de> >> >> The unixctl implementation in Python has been split into three parts in >> the past. During this process the UnixctlClient was duplicated, in >> python/ovs/unixctl/client.py and python/ovs/unixctl/server.py. This >> patch removes the duplicate from the latter. >> >> Fixes: 53cf9963ccc6 ("python: Break unixctl implementation into re...") > Please, don't abbreviate tags, i.e. the Fixes tag should contain the full > commit name regardless of the length. > > Otherwise, the change seem fine. Missed that part in "Submitting patches" guide. Thank you for pointing out! Patch v2 is out: https://patchwork.ozlabs.org/project/openvswitch/patch/20231030090259.15251-2-jmeng@redhat.com/
diff --git a/python/ovs/unixctl/server.py b/python/ovs/unixctl/server.py index 5f9b3e739..b9cb52fad 100644 --- a/python/ovs/unixctl/server.py +++ b/python/ovs/unixctl/server.py @@ -211,47 +211,3 @@ class UnixctlServer(object): version) return 0, UnixctlServer(listener) - - -class UnixctlClient(object): - def __init__(self, conn): - assert isinstance(conn, ovs.jsonrpc.Connection) - self._conn = conn - - def transact(self, command, argv): - assert isinstance(command, str) - assert isinstance(argv, list) - for arg in argv: - assert isinstance(arg, str) - - request = Message.create_request(command, argv) - error, reply = self._conn.transact_block(request) - - if error: - vlog.warn("error communicating with %s: %s" - % (self._conn.name, os.strerror(error))) - return error, None, None - - if reply.error is not None: - return 0, str(reply.error), None - else: - assert reply.result is not None - return 0, None, str(reply.result) - - def close(self): - self._conn.close() - self.conn = None - - @staticmethod - def create(path): - assert isinstance(path, str) - - unix = "unix:%s" % ovs.util.abs_file_name(ovs.dirs.RUNDIR, path) - error, stream = ovs.stream.Stream.open_block( - ovs.stream.Stream.open(unix)) - - if error: - vlog.warn("failed to connect to %s" % path) - return error, None - - return 0, UnixctlClient(ovs.jsonrpc.Connection(stream))