[ 
https://issues.apache.org/jira/browse/TINKERPOP-1267?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15265146#comment-15265146
 ] 

ASF GitHub Bot commented on TINKERPOP-1267:
-------------------------------------------

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
    ```


> Configure Console for no timeout on remote requests
> ---------------------------------------------------
>
>                 Key: TINKERPOP-1267
>                 URL: https://issues.apache.org/jira/browse/TINKERPOP-1267
>             Project: TinkerPop
>          Issue Type: Improvement
>          Components: console
>    Affects Versions: 3.1.2-incubating
>            Reporter: stephen mallette
>            Assignee: stephen mallette
>            Priority: Minor
>             Fix For: 3.1.3
>
>
> The console comes with a default timeout of 3 minutes for remote requests to 
> Gremlin Server. You can change that value with:
> {code}
> :remote config timeout 60000
> {code} 
> or you can make it "basically" indefinite with:
> {code}
> :remote config timeout max
> {code} 
> I think "max" is kinda weird now that I look at it. That basically sets a 
> time out for {{Integer.MAX_VALUE}} when you really just want to wait 
> indefinitely and have no timeout at all. I guess "max" is just a sort of a 
> bad word. It seems like it would be good to deprecate "max" in favor of:
> {code}
> :remote config timeout none
> {code} 
> which is more in line with what someone actually wants to have when they are 
> doing "max".
> Change the default timeout from 3 minutes to "none". It seems to be the more 
> expected default. Can't think of a circumstance in the context of the 
> console, where the client shouldn't just wait for the server's response.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to