You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by GitBox <gi...@apache.org> on 2021/04/07 22:24:12 UTC
[GitHub] [tinkerpop] lyndonb-bq opened a new pull request #1415: TINKERPOP-2546 Gremlin-Python Client Transport Layer to use AIOHTTP
lyndonb-bq opened a new pull request #1415:
URL: https://github.com/apache/tinkerpop/pull/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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] lyndonb-bq closed pull request #1415: TINKERPOP-2546 Gremlin-Python Client Transport Layer to use AIOHTTP
Posted by GitBox <gi...@apache.org>.
lyndonb-bq closed pull request #1415:
URL: https://github.com/apache/tinkerpop/pull/1415
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on pull request #1415: TINKERPOP-2546 Gremlin-Python Client Transport Layer to use AIOHTTP
Posted by GitBox <gi...@apache.org>.
spmallette commented on pull request #1415:
URL: https://github.com/apache/tinkerpop/pull/1415#issuecomment-816702280
There are a few places in the documentation that make mention of tornado - they all seem to be in the "Python" section here:
https://github.com/apache/tinkerpop/blob/master/docs/src/reference/gremlin-variants.asciidoc#gremlin-python
If you could find those and make adjustments as necessary that would be great.
Also, this sort of change is the kind of thing we like to callout in our Upgrade Documentation, where we let folks know about major sorts of changes in new versions. If you'd like to draft something there in this PR that would be nice otherwise I can just take care of it after merge:
https://github.com/apache/tinkerpop/blob/master/docs/src/upgrade/release-3.5.x.asciidoc
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on a change in pull request #1415: TINKERPOP-2546 Gremlin-Python Client Transport Layer to use AIOHTTP
Posted by GitBox <gi...@apache.org>.
spmallette commented on a change in pull request #1415:
URL: https://github.com/apache/tinkerpop/pull/1415#discussion_r611496213
##########
File path: docs/src/upgrade/release-3.5.x.asciidoc
##########
@@ -494,14 +494,23 @@ See: link:https://issues.apache.org/jira/browse/TINKERPOP-2517[TINKERPOP-2517]
==== Python 2.x Support
-The gremlinpython module no longer supports Python 2.x. Users must use Python 3 going forward. For the most part, from
+The gremlin-python module no longer supports Python 2.x. Users must use Python 3 going forward. For the most part, from
a user's perspective, there are no specific API changes to consider as a result of this change. It is also worth
noting that Jython support has been removed and that `gremlin-python` no longer produces a JVM-based artifact. This
change means that the `GremlinJythonScriptEngine` no longer exists and there is no way to write native Python lambdas.
All lambdas should be written using `gremlin-groovy` if they are needed.
See: link:https://issues.apache.org/jira/browse/TINKERPOP-2317[TINKERPOP-2317]
+==== Python Transportation Layer Rewrite
Review comment:
This is good - two things:
1. please include the "See: " to link to the JIRA
2. i think we should clearly state that Tornado support has been removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] lyndonb-bq commented on pull request #1415: TINKERPOP-2546 Gremlin-Python Client Transport Layer to use AIOHTTP
Posted by GitBox <gi...@apache.org>.
lyndonb-bq commented on pull request #1415:
URL: https://github.com/apache/tinkerpop/pull/1415#issuecomment-819001271
Closing this because I made a new pull request with cleaned up git history:
https://github.com/apache/tinkerpop/pull/1416
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] lyndonb-bq commented on pull request #1415: TINKERPOP-2546 Gremlin-Python Client Transport Layer to use AIOHTTP
Posted by GitBox <gi...@apache.org>.
lyndonb-bq commented on pull request #1415:
URL: https://github.com/apache/tinkerpop/pull/1415#issuecomment-816865395
> There are a few places in the documentation that make mention of tornado - they all seem to be in the "Python" section here:
>
> https://github.com/apache/tinkerpop/blob/master/docs/src/reference/gremlin-variants.asciidoc#gremlin-python
>
> If you could find those and make adjustments as necessary that would be great.
>
> Also, this sort of change is the kind of thing we like to callout in our Upgrade Documentation, where we let folks know about major sorts of changes in new versions. If you'd like to draft something there in this PR that would be nice otherwise I can just take care of it after merge:
>
> https://github.com/apache/tinkerpop/blob/master/docs/src/upgrade/release-3.5.x.asciidoc
Thanks for the feedback @spmallette, sorry I missed those references on the first pass. I have corrected them. Also I added the upgrade documentation, if you want me to add any more or less on it, please let me know!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org