Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

TINKERPOP-1761: Cancel script evaluation timeout #709

Merged
merged 1 commit into from
Oct 12, 2017

Conversation

ko-mueller
Copy link
Contributor

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

I basically added the code again which was wrongly removed in TINKERPOP-1714, plus added a test. All unit tests passed and I confirmed manually that this fixes the issue I experienced.

@dkuppitz
Copy link
Contributor

VOTE: +1

@spmallette
Copy link
Contributor

@ko-mueller thanks for submitting this. I just wanted to let you know that it may take some time to get this merged. Every time this area of code gets changed there is suffering, so I'd like to be sure adding this code back doesn't create some new subtle issue that we solved with TINKERPOP-1714.

@ko-mueller
Copy link
Contributor Author

Alright, it just would be nice to have this fixed in the next release. I didn't upgrade from 3.2.5 to 3.3.0 because of this. It's not a big issue for me right now, just that I'd like to stay current (without implementing a work-around for this).

@spmallette
Copy link
Contributor

@BrynCooke i've been looking at this on and off since it was submitted and i can't come up with any concrete reason not to vote to merge this. i think we still get to release the {{evaluationFuture}} for GC with this added blob of code that handles cancellation of the cancellation. any last words?

@robertdale
Copy link
Member

Looks good.
VOTE +1

@spmallette
Copy link
Contributor

All tests pass with docker/build.sh -t -n -i

VOTE +1

@asfgit asfgit merged commit 2a8c92f into apache:tp32 Oct 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants