You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by spmallette <gi...@git.apache.org> on 2016/04/26 13:49:08 UTC
[GitHub] incubator-tinkerpop pull request: TINKERPOP-1267 Added option for ...
GitHub user spmallette opened a pull request:
https://github.com/apache/incubator-tinkerpop/pull/294
TINKERPOP-1267 Added option for "none" on remote timeouts.
Added some tests to validate the new timeout setting and did some manual tests:
```text
gremlin> :remote connect tinkerpop.server conf/remote.yaml
==>Connected - localhost/127.0.0.1:8182
gremlin> :remote config timeout max
==>Set remote timeout to 2147483647ms
gremlin> :remote config timeout none
==>Remote timeout is disable
gremlin> :remote config timeout 10000
==>Set remote timeout to 10000ms
gremlin> :> Thread.sleep(9000)
==>null
gremlin> :> Thread.sleep(11000)
Request timed out while processing - increase the timeout with the :remote command
Display stack trace? [yN] n
```
Successful run of `mvn clean install && mvn verify -pl gremlin-console -DskipIntegrationTests=false`
VOTE +1
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/incubator-tinkerpop TINKERPOP-1267
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-tinkerpop/pull/294.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #294
----
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP-1267 Added option for ...
Posted by pluradj <gi...@git.apache.org>.
Github user pluradj commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/294#issuecomment-215934896
Other than that nit, everything else checked out ok.
The fix for that is trivial, so I'll go ahead and vote.
I inspected the code, reviewed the docs, ran the tests.
VOTE: +1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP-1267 Added option for ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-tinkerpop/pull/294
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP-1267 Added option for ...
Posted by pluradj <gi...@git.apache.org>.
Github user pluradj commented on a diff in the pull request:
https://github.com/apache/incubator-tinkerpop/pull/294#discussion_r61664727
--- Diff: gremlin-console/src/main/java/org/apache/tinkerpop/gremlin/console/groovy/plugin/DriverRemoteAcceptor.java ---
@@ -104,13 +113,14 @@ public Object configure(final List<String> args) throws RemoteException {
final List<String> arguments = args.subList(1, args.size());
if (option.equals(TOKEN_TIMEOUT)) {
- final String errorMessage = "The timeout option expects a positive integer representing milliseconds or 'max' as an argument";
+ final String errorMessage = "The timeout option expects a positive integer representing milliseconds or 'none' as an argument";
if (arguments.size() != 1) throw new RemoteException(errorMessage);
try {
- final int to = arguments.get(0).equals(TOKEN_MAX) ? Integer.MAX_VALUE : Integer.parseInt(arguments.get(0));
- if (to <= 0) throw new RemoteException(errorMessage);
- this.timeout = to;
- return "Set remote timeout to " + to + "ms";
+ // first check for MAX timeout then NONE and finally parse the config to int. "max" is now "deprecated"
+ // in the sense that it will no longer be promoted. support for it will be removed at a later date
+ timeout = arguments.get(0).equals(TOKEN_MAX) ? Integer.MAX_VALUE :
+ arguments.get(0).equals(TOKEN_NONE) ? NO_TIMEOUT : Integer.parseInt(arguments.get(0));
+ return timeout == NO_TIMEOUT ? "Remote timeout is disable" : "Set remote timeout to " + timeout + "ms";
--- End diff --
Missing the check here for `if (timeout < 0) throw new RemoteException(errorMessage);` so you can end up with
```
gremlin> :remote config timeout -2
==>Set remote timeout to -2ms
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP-1267 Added option for ...
Posted by dkuppitz <gi...@git.apache.org>.
Github user dkuppitz commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/294#issuecomment-214851656
Standard tests: passed
Integration tests: passed
User docs: successfully built
VOTE: +1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---