mbox series

[ovs-dev,v4,0/2] Partial cluster support in Python IDL client

Message ID 20180807113719.2959-1-nusiddiq@redhat.com
Headers show
Series Partial cluster support in Python IDL client | expand

Message

Numan Siddique Aug. 7, 2018, 11:37 a.m. UTC
From: Numan Siddique <nusiddiq@redhat.com>

Python IDL library is lacking the functionality to connect to the
clustered db servers by providing multiple remotes (like -
"tcp:10.0.0.1:6641, tcp:10.0.0.2:6641, tcp:10.0.0.3:6641") in the
connection string.

This patch adds this functionality to the python idl library.
It still lacks the feature to connect to the master of the cluster.
To add this
  - python idl client should monitor and read the '_Server' schema
  - connect to the master of the cluster.

I will submit the patch once that is ready. But for now I think this
is good enough for the clients to connect to the cluster dbs.


v3 -> v4
--------
p1 -> As per Ben's suggestion, used the select.poll() to 
know the connection status. In case eventlet/gevent is used and 
select.poll is monkey patched, then get the original select.poll()
using eventlet.patcher.original/gevent.monkey.get_original
functions.

v2 -> v3
--------
Addressed the review comments from Ben to parse the remote in
db/idl.py

v1 -> v2
--------
Deleted the debug code which I forgot to cleanup when sending v1.

Numan Siddique (2):
  ovs python: ovs.stream.open_block() returns success even if the remote
    is unreachable
  python jsonrpc: Allow jsonrpc_session to have more than one remote.

 python/ovs/db/idl.py      | 20 ++++++++++-
 python/ovs/jsonrpc.py     | 39 +++++++++++++++++-----
 python/ovs/poller.py      | 34 +++++++++++++++++--
 python/ovs/socket_util.py |  6 ++--
 python/ovs/stream.py      | 11 ++++--
 tests/automake.mk         |  1 +
 tests/ovsdb-idl.at        | 70 +++++++++++++++++++++++++++++++++++++++
 tests/test-ovsdb.py       | 13 ++++++--
 tests/test-stream.py      | 32 ++++++++++++++++++
 9 files changed, 208 insertions(+), 18 deletions(-)
 create mode 100644 tests/test-stream.py

Comments

Mark Michelson Aug. 10, 2018, 8:48 p.m. UTC | #1
Acked-by: Mark Michelson <mmichels@redhat.com>

Looks like you may have fixed an existing bug or two with this series, too.

On 08/07/2018 07:37 AM, nusiddiq@redhat.com wrote:
> From: Numan Siddique <nusiddiq@redhat.com>
> 
> Python IDL library is lacking the functionality to connect to the
> clustered db servers by providing multiple remotes (like -
> "tcp:10.0.0.1:6641, tcp:10.0.0.2:6641, tcp:10.0.0.3:6641") in the
> connection string.
> 
> This patch adds this functionality to the python idl library.
> It still lacks the feature to connect to the master of the cluster.
> To add this
>    - python idl client should monitor and read the '_Server' schema
>    - connect to the master of the cluster.
> 
> I will submit the patch once that is ready. But for now I think this
> is good enough for the clients to connect to the cluster dbs.
> 
> 
> v3 -> v4
> --------
> p1 -> As per Ben's suggestion, used the select.poll() to
> know the connection status. In case eventlet/gevent is used and
> select.poll is monkey patched, then get the original select.poll()
> using eventlet.patcher.original/gevent.monkey.get_original
> functions.
> 
> v2 -> v3
> --------
> Addressed the review comments from Ben to parse the remote in
> db/idl.py
> 
> v1 -> v2
> --------
> Deleted the debug code which I forgot to cleanup when sending v1.
> 
> Numan Siddique (2):
>    ovs python: ovs.stream.open_block() returns success even if the remote
>      is unreachable
>    python jsonrpc: Allow jsonrpc_session to have more than one remote.
> 
>   python/ovs/db/idl.py      | 20 ++++++++++-
>   python/ovs/jsonrpc.py     | 39 +++++++++++++++++-----
>   python/ovs/poller.py      | 34 +++++++++++++++++--
>   python/ovs/socket_util.py |  6 ++--
>   python/ovs/stream.py      | 11 ++++--
>   tests/automake.mk         |  1 +
>   tests/ovsdb-idl.at        | 70 +++++++++++++++++++++++++++++++++++++++
>   tests/test-ovsdb.py       | 13 ++++++--
>   tests/test-stream.py      | 32 ++++++++++++++++++
>   9 files changed, 208 insertions(+), 18 deletions(-)
>   create mode 100644 tests/test-stream.py
>
Numan Siddique Aug. 14, 2018, 5:33 p.m. UTC | #2
On Sat, Aug 11, 2018 at 2:19 AM Mark Michelson <mmichels@redhat.com> wrote:

> Acked-by: Mark Michelson <mmichels@redhat.com>
>
> Looks like you may have fixed an existing bug or two with this series, too.
>
>
Thanks for the reviews Mark. Yes, that's right, they were a couple of
issues which got fixed with this patch.

Thanks
Numan


> On 08/07/2018 07:37 AM, nusiddiq@redhat.com wrote:
> > From: Numan Siddique <nusiddiq@redhat.com>
> >
> > Python IDL library is lacking the functionality to connect to the
> > clustered db servers by providing multiple remotes (like -
> > "tcp:10.0.0.1:6641, tcp:10.0.0.2:6641, tcp:10.0.0.3:6641") in the
> > connection string.
> >
> > This patch adds this functionality to the python idl library.
> > It still lacks the feature to connect to the master of the cluster.
> > To add this
> >    - python idl client should monitor and read the '_Server' schema
> >    - connect to the master of the cluster.
> >
> > I will submit the patch once that is ready. But for now I think this
> > is good enough for the clients to connect to the cluster dbs.
> >
> >
> > v3 -> v4
> > --------
> > p1 -> As per Ben's suggestion, used the select.poll() to
> > know the connection status. In case eventlet/gevent is used and
> > select.poll is monkey patched, then get the original select.poll()
> > using eventlet.patcher.original/gevent.monkey.get_original
> > functions.
> >
> > v2 -> v3
> > --------
> > Addressed the review comments from Ben to parse the remote in
> > db/idl.py
> >
> > v1 -> v2
> > --------
> > Deleted the debug code which I forgot to cleanup when sending v1.
> >
> > Numan Siddique (2):
> >    ovs python: ovs.stream.open_block() returns success even if the remote
> >      is unreachable
> >    python jsonrpc: Allow jsonrpc_session to have more than one remote.
> >
> >   python/ovs/db/idl.py      | 20 ++++++++++-
> >   python/ovs/jsonrpc.py     | 39 +++++++++++++++++-----
> >   python/ovs/poller.py      | 34 +++++++++++++++++--
> >   python/ovs/socket_util.py |  6 ++--
> >   python/ovs/stream.py      | 11 ++++--
> >   tests/automake.mk         |  1 +
> >   tests/ovsdb-idl.at        | 70 +++++++++++++++++++++++++++++++++++++++
> >   tests/test-ovsdb.py       | 13 ++++++--
> >   tests/test-stream.py      | 32 ++++++++++++++++++
> >   9 files changed, 208 insertions(+), 18 deletions(-)
> >   create mode 100644 tests/test-stream.py
> >
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Ben Pfaff Aug. 14, 2018, 6:50 p.m. UTC | #3
On Tue, Aug 07, 2018 at 05:07:19PM +0530, nusiddiq@redhat.com wrote:
> From: Numan Siddique <nusiddiq@redhat.com>
> 
> Python IDL library is lacking the functionality to connect to the
> clustered db servers by providing multiple remotes (like -
> "tcp:10.0.0.1:6641, tcp:10.0.0.2:6641, tcp:10.0.0.3:6641") in the
> connection string.
> 
> This patch adds this functionality to the python idl library.
> It still lacks the feature to connect to the master of the cluster.
> To add this
>   - python idl client should monitor and read the '_Server' schema
>   - connect to the master of the cluster.
> 
> I will submit the patch once that is ready. But for now I think this
> is good enough for the clients to connect to the cluster dbs.

I applied this series to master.  Do you want it backported?
Numan Siddique Aug. 15, 2018, 9:09 a.m. UTC | #4
On Wed, Aug 15, 2018 at 12:20 AM Ben Pfaff <blp@ovn.org> wrote:

> On Tue, Aug 07, 2018 at 05:07:19PM +0530, nusiddiq@redhat.com wrote:
> > From: Numan Siddique <nusiddiq@redhat.com>
> >
> > Python IDL library is lacking the functionality to connect to the
> > clustered db servers by providing multiple remotes (like -
> > "tcp:10.0.0.1:6641, tcp:10.0.0.2:6641, tcp:10.0.0.3:6641") in the
> > connection string.
> >
> > This patch adds this functionality to the python idl library.
> > It still lacks the feature to connect to the master of the cluster.
> > To add this
> >   - python idl client should monitor and read the '_Server' schema
> >   - connect to the master of the cluster.
> >
> > I will submit the patch once that is ready. But for now I think this
> > is good enough for the clients to connect to the cluster dbs.
>
> I applied this series to master.  Do you want it backported?
>

Thanks for the review and applying the patches. Yes. It would be great if
it is back ported to
branch 2.10.

Numan
Ben Pfaff Aug. 15, 2018, 5:19 p.m. UTC | #5
On Wed, Aug 15, 2018 at 02:39:42PM +0530, Numan Siddique wrote:
> On Wed, Aug 15, 2018 at 12:20 AM Ben Pfaff <blp@ovn.org> wrote:
> 
> > On Tue, Aug 07, 2018 at 05:07:19PM +0530, nusiddiq@redhat.com wrote:
> > > From: Numan Siddique <nusiddiq@redhat.com>
> > >
> > > Python IDL library is lacking the functionality to connect to the
> > > clustered db servers by providing multiple remotes (like -
> > > "tcp:10.0.0.1:6641, tcp:10.0.0.2:6641, tcp:10.0.0.3:6641") in the
> > > connection string.
> > >
> > > This patch adds this functionality to the python idl library.
> > > It still lacks the feature to connect to the master of the cluster.
> > > To add this
> > >   - python idl client should monitor and read the '_Server' schema
> > >   - connect to the master of the cluster.
> > >
> > > I will submit the patch once that is ready. But for now I think this
> > > is good enough for the clients to connect to the cluster dbs.
> >
> > I applied this series to master.  Do you want it backported?
> >
> 
> Thanks for the review and applying the patches. Yes. It would be great if
> it is back ported to
> branch 2.10.

Done.