You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by Lyndon Bauto <ly...@bitquilltech.com> on 2021/04/01 20:43:45 UTC

[DISCUSS] Gremlin-Python Client Transport Layer to use AIOHTTP

Hi all,

I am looking to rewrite the current transport layer of the Gremlin-Python
client to use AIOHTTP [1] instead of Tornado. My plan is to keep the
current abstract base class the same and simply rewrite the underlying
functions to utilize AIOHTTP.

My plan is to use AIOHTTP v3.7.x, which supports Python 3.6+. This will
deprecate support of older Python versions, but the current docker Python
version for the repository is 3.6, so this should not pose a problem.

I have done some prototyping and testing and found the rewrite fixed the
following errors:

   - Event loop issues when using Tornado [2]
   - Heartbeat timeout support [3]
   - Multithreaded DriverRemoteConnection [4]
   - Build asyncio task errors [5]


Because my plan is to rewrite the transport layer using the same abstract
base class, we have the option of keeping the old Tornado library as is and
deprecating it while making the AIOHTTP implementation the default
implementation. Otherwise we can fully remove it. I am fine with either
option but wanted to get some other opinions on it.

Please let me know your thoughts around this and I will take it all into
consideration before generating a pull request.

[1] https://github.com/aio-libs/aiohttp
[2] https://github.com/jupyter/notebook/issues/3397
[3] https://issues.apache.org/jira/browse/TINKERPOP-1886
[4] https://issues.apache.org/jira/browse/TINKERPOP-2388
[5] https://issues.apache.org/jira/browse/TINKERPOP-2484

-- 
*Lyndon Bauto*
Senior Software Developer
Bit Quill Technologies Inc.
lyndonb@bitquilltech.com
https://www.bitquilltech.com

Re: [DISCUSS] Gremlin-Python Client Transport Layer to use AIOHTTP

Posted by Stephen Mallette <sp...@gmail.com>.
Thanks for the input, Dave. I suppose my thinking at this stage is that
given the window to 3.5.0 (in a few weeks) I'd be concerned about a major
shift in how gremlin-python works. Lyndon's solution is quite surgical and
seems to carry little risk. I'd be in favor of a pure aiohttp approach in a
future release though for sure.

On Fri, Apr 2, 2021 at 11:19 AM David Brown <da...@gmail.com> wrote:

> You guys should just switch to a real Asyncio (aiohttp) client
> implementation that actually leverages the async/await syntax. As Stephen
> mentioned I already did it with aiogremlin if you want to see an example. I
> don't control the repo anymore but there is an example implementation there
> with an async GLV as well: https://github.com/goblin-ogm/aiogremlin
>
>
>
> On Fri, Apr 2, 2021 at 4:17 AM Stephen Mallette <sp...@gmail.com>
> wrote:
>
> > This is an important body of work and I think it is being done in the
> right
> > fashion. I had thought that we would completely remove the Transport
> > abstraction but since "it works" it seems as though there is no need at
> > this point. I do seem to recall that David Brown had this abstraction in
> > play with aoihttp in mind because aoigremlin was also developed but we
> > couldn't use the work here because of our endless dependence on Python 2.
> > With Python 2 gone in 3.5.0 we are free from that requirement.
> >
> > As for keeping Tornado around, I'm on the fence to some degree. I like
> > keeping the idea of keeping legacy code around for a bit between versions
> > but the problem in this case is that Tornado doesn't really work in
> 3.5.0.
> > It just has too many problems. i think I'm leaning toward simply removing
> > it. Please voice concerns for removing Tornado in the next 72 hours
> (April
> > 5, 2021, 7:15am ET) otherwise I think Lyndon should proceed with that
> path.
> >
> >
> >
> > On Thu, Apr 1, 2021 at 4:45 PM Lyndon Bauto <ly...@bitquilltech.com>
> > wrote:
> >
> > > Hi all,
> > >
> > > I am looking to rewrite the current transport layer of the
> Gremlin-Python
> > > client to use AIOHTTP [1] instead of Tornado. My plan is to keep the
> > > current abstract base class the same and simply rewrite the underlying
> > > functions to utilize AIOHTTP.
> > >
> > > My plan is to use AIOHTTP v3.7.x, which supports Python 3.6+. This will
> > > deprecate support of older Python versions, but the current docker
> Python
> > > version for the repository is 3.6, so this should not pose a problem.
> > >
> > > I have done some prototyping and testing and found the rewrite fixed
> the
> > > following errors:
> > >
> > >    - Event loop issues when using Tornado [2]
> > >    - Heartbeat timeout support [3]
> > >    - Multithreaded DriverRemoteConnection [4]
> > >    - Build asyncio task errors [5]
> > >
> > >
> > > Because my plan is to rewrite the transport layer using the same
> abstract
> > > base class, we have the option of keeping the old Tornado library as is
> > and
> > > deprecating it while making the AIOHTTP implementation the default
> > > implementation. Otherwise we can fully remove it. I am fine with either
> > > option but wanted to get some other opinions on it.
> > >
> > > Please let me know your thoughts around this and I will take it all
> into
> > > consideration before generating a pull request.
> > >
> > > [1] https://github.com/aio-libs/aiohttp
> > > [2] https://github.com/jupyter/notebook/issues/3397
> > > [3] https://issues.apache.org/jira/browse/TINKERPOP-1886
> > > [4] https://issues.apache.org/jira/browse/TINKERPOP-2388
> > > [5] https://issues.apache.org/jira/browse/TINKERPOP-2484
> > >
> > > --
> > > *Lyndon Bauto*
> > > Senior Software Developer
> > > Bit Quill Technologies Inc.
> > > lyndonb@bitquilltech.com
> > > https://www.bitquilltech.com
> > >
> >
>

Re: [DISCUSS] Gremlin-Python Client Transport Layer to use AIOHTTP

Posted by David Brown <da...@gmail.com>.
You guys should just switch to a real Asyncio (aiohttp) client
implementation that actually leverages the async/await syntax. As Stephen
mentioned I already did it with aiogremlin if you want to see an example. I
don't control the repo anymore but there is an example implementation there
with an async GLV as well: https://github.com/goblin-ogm/aiogremlin



On Fri, Apr 2, 2021 at 4:17 AM Stephen Mallette <sp...@gmail.com>
wrote:

> This is an important body of work and I think it is being done in the right
> fashion. I had thought that we would completely remove the Transport
> abstraction but since "it works" it seems as though there is no need at
> this point. I do seem to recall that David Brown had this abstraction in
> play with aoihttp in mind because aoigremlin was also developed but we
> couldn't use the work here because of our endless dependence on Python 2.
> With Python 2 gone in 3.5.0 we are free from that requirement.
>
> As for keeping Tornado around, I'm on the fence to some degree. I like
> keeping the idea of keeping legacy code around for a bit between versions
> but the problem in this case is that Tornado doesn't really work in 3.5.0.
> It just has too many problems. i think I'm leaning toward simply removing
> it. Please voice concerns for removing Tornado in the next 72 hours (April
> 5, 2021, 7:15am ET) otherwise I think Lyndon should proceed with that path.
>
>
>
> On Thu, Apr 1, 2021 at 4:45 PM Lyndon Bauto <ly...@bitquilltech.com>
> wrote:
>
> > Hi all,
> >
> > I am looking to rewrite the current transport layer of the Gremlin-Python
> > client to use AIOHTTP [1] instead of Tornado. My plan is to keep the
> > current abstract base class the same and simply rewrite the underlying
> > functions to utilize AIOHTTP.
> >
> > My plan is to use AIOHTTP v3.7.x, which supports Python 3.6+. This will
> > deprecate support of older Python versions, but the current docker Python
> > version for the repository is 3.6, so this should not pose a problem.
> >
> > I have done some prototyping and testing and found the rewrite fixed the
> > following errors:
> >
> >    - Event loop issues when using Tornado [2]
> >    - Heartbeat timeout support [3]
> >    - Multithreaded DriverRemoteConnection [4]
> >    - Build asyncio task errors [5]
> >
> >
> > Because my plan is to rewrite the transport layer using the same abstract
> > base class, we have the option of keeping the old Tornado library as is
> and
> > deprecating it while making the AIOHTTP implementation the default
> > implementation. Otherwise we can fully remove it. I am fine with either
> > option but wanted to get some other opinions on it.
> >
> > Please let me know your thoughts around this and I will take it all into
> > consideration before generating a pull request.
> >
> > [1] https://github.com/aio-libs/aiohttp
> > [2] https://github.com/jupyter/notebook/issues/3397
> > [3] https://issues.apache.org/jira/browse/TINKERPOP-1886
> > [4] https://issues.apache.org/jira/browse/TINKERPOP-2388
> > [5] https://issues.apache.org/jira/browse/TINKERPOP-2484
> >
> > --
> > *Lyndon Bauto*
> > Senior Software Developer
> > Bit Quill Technologies Inc.
> > lyndonb@bitquilltech.com
> > https://www.bitquilltech.com
> >
>

Re: [DISCUSS] Gremlin-Python Client Transport Layer to use AIOHTTP

Posted by Stephen Mallette <sp...@gmail.com>.
This is an important body of work and I think it is being done in the right
fashion. I had thought that we would completely remove the Transport
abstraction but since "it works" it seems as though there is no need at
this point. I do seem to recall that David Brown had this abstraction in
play with aoihttp in mind because aoigremlin was also developed but we
couldn't use the work here because of our endless dependence on Python 2.
With Python 2 gone in 3.5.0 we are free from that requirement.

As for keeping Tornado around, I'm on the fence to some degree. I like
keeping the idea of keeping legacy code around for a bit between versions
but the problem in this case is that Tornado doesn't really work in 3.5.0.
It just has too many problems. i think I'm leaning toward simply removing
it. Please voice concerns for removing Tornado in the next 72 hours (April
5, 2021, 7:15am ET) otherwise I think Lyndon should proceed with that path.



On Thu, Apr 1, 2021 at 4:45 PM Lyndon Bauto <ly...@bitquilltech.com>
wrote:

> Hi all,
>
> I am looking to rewrite the current transport layer of the Gremlin-Python
> client to use AIOHTTP [1] instead of Tornado. My plan is to keep the
> current abstract base class the same and simply rewrite the underlying
> functions to utilize AIOHTTP.
>
> My plan is to use AIOHTTP v3.7.x, which supports Python 3.6+. This will
> deprecate support of older Python versions, but the current docker Python
> version for the repository is 3.6, so this should not pose a problem.
>
> I have done some prototyping and testing and found the rewrite fixed the
> following errors:
>
>    - Event loop issues when using Tornado [2]
>    - Heartbeat timeout support [3]
>    - Multithreaded DriverRemoteConnection [4]
>    - Build asyncio task errors [5]
>
>
> Because my plan is to rewrite the transport layer using the same abstract
> base class, we have the option of keeping the old Tornado library as is and
> deprecating it while making the AIOHTTP implementation the default
> implementation. Otherwise we can fully remove it. I am fine with either
> option but wanted to get some other opinions on it.
>
> Please let me know your thoughts around this and I will take it all into
> consideration before generating a pull request.
>
> [1] https://github.com/aio-libs/aiohttp
> [2] https://github.com/jupyter/notebook/issues/3397
> [3] https://issues.apache.org/jira/browse/TINKERPOP-1886
> [4] https://issues.apache.org/jira/browse/TINKERPOP-2388
> [5] https://issues.apache.org/jira/browse/TINKERPOP-2484
>
> --
> *Lyndon Bauto*
> Senior Software Developer
> Bit Quill Technologies Inc.
> lyndonb@bitquilltech.com
> https://www.bitquilltech.com
>