-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23678/#review48131
-----------------------------------------------------------



src/c/src/zookeeper.c
<https://reviews.apache.org/r/23678/#comment84422>

    this constant could use a comment or a better name, i think. maybe 
MIN_ALLOWED_SESSION_TIMEOUT or at least a comment as to where in the protocol 
this is defined/mandated?



src/java/main/org/apache/zookeeper/ClientCnxn.java
<https://reviews.apache.org/r/23678/#comment84423>

    DEFAULT_MIN_TIMEOUT is misleading, maybe MIN_ALLOW_SESSION_TIMEOUT as well? 
or something along those lines


- Raul Gutierrez Segales


On July 18, 2014, 1:22 p.m., Bill Havanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23678/
> -----------------------------------------------------------
> 
> (Updated July 18, 2014, 1:22 p.m.)
> 
> 
> Review request for zookeeper.
> 
> 
> Bugs: ZOOKEEPER-776
>     https://issues.apache.org/jira/browse/ZOOKEEPER-776
> 
> 
> Repository: zookeeper-git
> 
> 
> Description
> -------
> 
> Addition of a check that the client session timeout is at least at a minimum. 
> The work here is based on the original patches from Gregory Haskins.
> 
> * In Java, the minimum is controlled by the zookeeper.minTimeout system 
> property, with default of 100.
> * In C, the minimum is fixed at 100.
> 
> Room for improvement (suggestions and assistance most welcome):
> 
> * There is no new unit test for the C client.
> * The minimum in the C client is not configurable.
> * The minimum enforcement is not yet documented.
> 
> 
> Diffs
> -----
> 
>   src/c/src/zookeeper.c 782ac52 
>   src/java/main/org/apache/zookeeper/ClientCnxn.java b4ece07 
>   src/java/test/org/apache/zookeeper/ClientCnxnTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/23678/diff/
> 
> 
> Testing
> -------
> 
> Compiles, new unit test passes.
> 
> 
> Thanks,
> 
> Bill Havanki
> 
>

Reply via email to