TINKERPOP-2546 Gremlin-Python Client Transport Layer to use AIOHTTP

[lyndonbauto]

Switching branch to fix commit history, old pull request is here: #1415

https://issues.apache.org/jira/browse/TINKERPOP-2546

Motivation

The current Tornado implementation of gremlin-python has a number of issues including heartbeat timeout, multithreading issues with DriverRemoteConnection, errors in the build due to the IOLoop, and issues with being unable to use the asyncio event loop when using the driver.

Because Python 2 support is dropped for Tinkerpop 3.5, this is a good time to switch to another implementation and do a clean cut of all these issues. This implementation uses AIOHTTP, this will also put gremlin-python one step closer to having a fully asyncio compatible solution that properly leverages the async/await syntax.

See more on the discussion here https://lists.apache.org/thread.html/rc4d047ba0245fde3261bad0ea156a0f35db1ab0f92c8e80d4fb1b815%40%3Cdev.tinkerpop.apache.org%3E

Changes

The following changes were made:

• Removed Tornado
• Added AIOHTTP implementation
• Added dependency on AIOHTTP and removed dependency on Tornado
• Added max_content_length and heartbeat parameters to transport
• Removed compression_options parameter from transport
• Added some unit tests
• Switched transport.closed to abc.abstractmethod/@property from @abstractproperty since the latter is deprecated

Applicable Tickets

Ticket for change to be made https://issues.apache.org/jira/browse/TINKERPOP-2546

The following issues are fixed with these changes:

• Heartbeat timeout issue: https://issues.apache.org/jira/browse/TINKERPOP-1886
• DriverRemoteConnection multithreading issue: https://issues.apache.org/jira/browse/TINKERPOP-2388
• IOLoop close build errors: https://issues.apache.org/jira/browse/TINKERPOP-2484

Testing Consideration

The following tests were done

• Builds with mvn clean install -pl gremlin-python
• Manually tested that if idleConnectionTimeout is set and a heartbeat is provided, connection does not drop
• Manually tested that if idleConnectionTimeout is set and a heartbeat is NOT provided, connection does drop
• Observed that the build output no longer prints IOLoop errors

[spmallette]

i recall you saying that the compression dict map was removed as there was not quite an analogous configuration for aiohttp but i though that there was a compression option in aiohttp that could be enabled as a flag still but i didn't see that here. am i missing where that is somehow?

[lyndonbauto]

i recall you saying that the compression dict map was removed as there was not quite an analogous configuration for aiohttp but i though that there was a compression option in aiohttp that could be enabled as a flag still but i didn't see that here. am i missing where that is somehow?

@spmallette There is a compression you can set for it but from what I have read it is a different type than Tornado's. I tried setting it up and all the tests failed whenever I used it so I don't think it is compatible with the current Gremlin server. I looked at aiogremlin's driver and found they didn't have it setup there either so I thought it might just not work for this use case unfortunately.

[lyndonbauto]

I updated the changes to allow keyword arg passthrough directly. This way a user can just directly pass through anything available in https://docs.aiohttp.org/en/stable/client_reference.html#aiohttp.ClientSession.ws_connect
I also mapped ssl_options to ssl and max_content_length to max_msg_size for backwards compatibility if users already have those being set.

[krlawrence]

I have run a lot of tests using this code as a replacement for the Tornado based version and with the last few additional commits have found no additional issues. We have had a lot of really tricky event loop conflict issues with Tornado and systems like Jupyter. These changes should alleviate that situation significantly.

VOTE +1

[spmallette]

VOTE +1 - i plan to merge this tomorrow at some point. it's been open for a while with lots of discussion, so i would think that if someone had concerns they would have been raised already. thanks