Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: timeout issues in gremlinpython==3.4.9 #17

Merged
merged 2 commits into from
Jul 16, 2021
Merged

fix: timeout issues in gremlinpython==3.4.9 #17

merged 2 commits into from
Jul 16, 2021

Conversation

rajat499
Copy link
Contributor

@rajat499 rajat499 commented Jul 15, 2021

Summary of Changes

gremlinpython==3.4.9 has default timeout set to 30 seconds, which causes WebSockets to timeout without completing the requests. Fix has been taken from the below references.
Reference: https://groups.google.com/g/gremlin-users/c/K0EVG3T-UrM/m/51bfkOZOCgAJ
apache/tinkerpop@3f93fdc
This is likely fixed in gremlinpython==3.5.0 but since we are using 3.4.9 this fix is required. We can't use gremlinpython==3.5.0 for now. Please refer: #16
A similar PR was merged in metadata service for the same issue: amundsen-io/amundsen#1334

CheckList

Make sure you have checked all steps below to ensure a timely review.

  • PR title addresses the issue accurately and concisely. Example: "Updates the version of Flask to v1.0.2"
  • PR includes a summary of changes.

Signed-off-by: Rajat Jaiswal <rajat.jaiswal@hotstar.com>
Copy link
Contributor

@friendtocephalopods friendtocephalopods left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll need to bump the version to use this change downstream, but seems reasonable to me!

@feng-tao
Copy link
Member

@rajat499 could you bump the version as suggested by @friendtocephalopods ?

Signed-off-by: Rajat Jaiswal <rajat.jaiswal@hotstar.com>
@rajat499
Copy link
Contributor Author

@feng-tao Sure. Done.

@rajat499
Copy link
Contributor Author

@feng-tao @friendtocephalopods I just checked that the timeout issue was fixed in v3.4.10 of gremlinpython. Reference: https://github.com/apache/tinkerpop/blob/master/CHANGELOG.asciidoc#tinkerpop-3410-release-date-january-18-2021
This version also doesn't run into issues as mentioned at: #16
I checked it on local. Should I squash the first commit and just bump up the version of gremlinpython from 3.4.9 to 3.4.10

@feng-tao feng-tao merged commit 4d8e77c into amundsen-io:master Jul 16, 2021
@feng-tao
Copy link
Member

@feng-tao @friendtocephalopods I just checked that the timeout issue was fixed in v3.4.10 of gremlinpython. Reference: https://github.com/apache/tinkerpop/blob/master/CHANGELOG.asciidoc#tinkerpop-3410-release-date-january-18-2021
This version also doesn't run into issues as mentioned at: #16
I checked it on local. Should I squash the first commit and just bump up the version of gremlinpython from 3.4.9 to 3.4.10

is there any api change from 3.4.9 to 3.4.10?

@rajat499 rajat499 deleted the fix-gremlin-timeout branch July 16, 2021 05:55
@rajat499
Copy link
Contributor Author

@feng-tao @friendtocephalopods I just checked that the timeout issue was fixed in v3.4.10 of gremlinpython. Reference: https://github.com/apache/tinkerpop/blob/master/CHANGELOG.asciidoc#tinkerpop-3410-release-date-january-18-2021
This version also doesn't run into issues as mentioned at: #16
I checked it on local. Should I squash the first commit and just bump up the version of gremlinpython from 3.4.9 to 3.4.10

is there any api change from 3.4.9 to 3.4.10?

No, there isn't any change in the API. They have added some new functionalities for java but nothing for python.
Ref: apache/tinkerpop@3.4.9...3.4.10

@feng-tao
Copy link
Member

@feng-tao @friendtocephalopods I just checked that the timeout issue was fixed in v3.4.10 of gremlinpython. Reference: https://github.com/apache/tinkerpop/blob/master/CHANGELOG.asciidoc#tinkerpop-3410-release-date-january-18-2021
This version also doesn't run into issues as mentioned at: #16
I checked it on local. Should I squash the first commit and just bump up the version of gremlinpython from 3.4.9 to 3.4.10

is there any api change from 3.4.9 to 3.4.10?

No, there isn't any change in the API. They have added some new functionalities for java but nothing for python.
Ref: apache/tinkerpop@3.4.9...3.4.10

cool, make sense to upgrade then.

@rajat499 rajat499 mentioned this pull request Jul 16, 2021
2 tasks
amommendes pushed a commit to amommendes/amundsengremlin that referenced this pull request Jul 22, 2021
* Fix timeout issues in tornado

Signed-off-by: Rajat Jaiswal <rajat.jaiswal@hotstar.com>

* Bump the version

Signed-off-by: Rajat Jaiswal <rajat.jaiswal@hotstar.com>
Signed-off-by: Amom <amommendes@hotmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants