You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by Rakesh R <ra...@huawei.com> on 2014/03/12 05:17:48 UTC

Review Request 19089: Inconsistent behavior in autocreation of dataDir and dataLogDir

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

Review request for zookeeper, fpj, michim, and Raul Gutierrez Segales.


Bugs: ZOOKEEPER-1878
    https://issues.apache.org/jira/browse/ZOOKEEPER-1878


Repository: zookeeper


Description
-------

See ZOOKEEPER-1878


Diffs
-----

  ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1566210 
  ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java 1566210 

Diff: https://reviews.apache.org/r/19089/diff/


Testing
-------

Test included


Thanks,

Rakesh R

Re: Review Request 19089: Inconsistent behavior in autocreation of dataDir and dataLogDir

Posted by Rakesh R <ra...@huawei.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19089/#review36951
-----------------------------------------------------------



./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java
<https://reviews.apache.org/r/19089/#comment68140>

    I couldn't see commons-io dependency.
    
    I've used the same pattern which already exists in the test case, is that fine for you?


- Rakesh R


On March 12, 2014, 4:17 a.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19089/
> -----------------------------------------------------------
> 
> (Updated March 12, 2014, 4:17 a.m.)
> 
> 
> Review request for zookeeper, fpj, michim, and Raul Gutierrez Segales.
> 
> 
> Bugs: ZOOKEEPER-1878
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1878
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> See ZOOKEEPER-1878
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1566210 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java 1566210 
> 
> Diff: https://reviews.apache.org/r/19089/diff/
> 
> 
> Testing
> -------
> 
> Test included
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 19089: Inconsistent behavior in autocreation of dataDir and dataLogDir

Posted by Raul Gutierrez Segales <rg...@itevenworks.net>.

> On March 12, 2014, 7:27 p.m., Camille Fournier wrote:
> > ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java, line 82
> > <https://reviews.apache.org/r/19089/diff/1/?file=517046#file517046line82>
> >
> >     Looks like the apache commons dep is only for the releaseaudit workflow, not for the project in general
> 
> Raul Gutierrez Segales wrote:
>     what about things like:
>     
>     https://github.com/apache/zookeeper/blob/trunk/src/java/main/org/apache/zookeeper/cli/CliCommand.java
>     
>     i see:
>     
>     ```
>     import org.apache.commons...
>     ```
>     
>     there.
> 
> Raul Gutierrez Segales wrote:
>     just asking to learn what the policy is, i am totally fine with not using it for this simple case (but if we can, sure it makes it nicer).
> 
> Rakesh R wrote:
>     I hope you are asking about the way we have used the commons-cli.jar ?
>     
>     Its for ZK shell commands - this simplifies the parsing logic of command line options.
>     Here first define the 'options' supported in my command. Now when user invokes the command, will just pass the 'options' and 'args' to the parser, rest he will take care for me.
>     Also, there are many other useful apis. Please have a look at CreateCommand.java
>     
>     I've seen many apache projects has adopted this approach.

Oh, no no - I was asking why it was okey to use the apache commons stuff for that case (for the CLI parsing) and not from within test cases.  I just didn't grok what Camille meant that it could only be used for the releaseaudit workflow. Thanks for answering tho :-)


- Raul


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


On March 12, 2014, 4:17 a.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19089/
> -----------------------------------------------------------
> 
> (Updated March 12, 2014, 4:17 a.m.)
> 
> 
> Review request for zookeeper, fpj, michim, and Raul Gutierrez Segales.
> 
> 
> Bugs: ZOOKEEPER-1878
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1878
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> See ZOOKEEPER-1878
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1566210 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java 1566210 
> 
> Diff: https://reviews.apache.org/r/19089/diff/
> 
> 
> Testing
> -------
> 
> Test included
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 19089: Inconsistent behavior in autocreation of dataDir and dataLogDir

Posted by Raul Gutierrez Segales <rg...@itevenworks.net>.

> On March 12, 2014, 7:27 p.m., Camille Fournier wrote:
> > ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java, line 82
> > <https://reviews.apache.org/r/19089/diff/1/?file=517046#file517046line82>
> >
> >     Looks like the apache commons dep is only for the releaseaudit workflow, not for the project in general

what about things like:

https://github.com/apache/zookeeper/blob/trunk/src/java/main/org/apache/zookeeper/cli/CliCommand.java

i see:

```
import org.apache.commons...
```

there. 


- Raul


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


On March 12, 2014, 4:17 a.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19089/
> -----------------------------------------------------------
> 
> (Updated March 12, 2014, 4:17 a.m.)
> 
> 
> Review request for zookeeper, fpj, michim, and Raul Gutierrez Segales.
> 
> 
> Bugs: ZOOKEEPER-1878
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1878
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> See ZOOKEEPER-1878
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1566210 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java 1566210 
> 
> Diff: https://reviews.apache.org/r/19089/diff/
> 
> 
> Testing
> -------
> 
> Test included
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 19089: Inconsistent behavior in autocreation of dataDir and dataLogDir

Posted by Raul Gutierrez Segales <rg...@itevenworks.net>.

> On March 12, 2014, 7:27 p.m., Camille Fournier wrote:
> > ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java, line 82
> > <https://reviews.apache.org/r/19089/diff/1/?file=517046#file517046line82>
> >
> >     Looks like the apache commons dep is only for the releaseaudit workflow, not for the project in general
> 
> Raul Gutierrez Segales wrote:
>     what about things like:
>     
>     https://github.com/apache/zookeeper/blob/trunk/src/java/main/org/apache/zookeeper/cli/CliCommand.java
>     
>     i see:
>     
>     ```
>     import org.apache.commons...
>     ```
>     
>     there.

just asking to learn what the policy is, i am totally fine with not using it for this simple case (but if we can, sure it makes it nicer). 


- Raul


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


On March 12, 2014, 4:17 a.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19089/
> -----------------------------------------------------------
> 
> (Updated March 12, 2014, 4:17 a.m.)
> 
> 
> Review request for zookeeper, fpj, michim, and Raul Gutierrez Segales.
> 
> 
> Bugs: ZOOKEEPER-1878
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1878
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> See ZOOKEEPER-1878
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1566210 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java 1566210 
> 
> Diff: https://reviews.apache.org/r/19089/diff/
> 
> 
> Testing
> -------
> 
> Test included
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 19089: Inconsistent behavior in autocreation of dataDir and dataLogDir

Posted by Rakesh R <ra...@huawei.com>.

> On March 12, 2014, 7:27 p.m., Camille Fournier wrote:
> > ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java, line 82
> > <https://reviews.apache.org/r/19089/diff/1/?file=517046#file517046line82>
> >
> >     Looks like the apache commons dep is only for the releaseaudit workflow, not for the project in general
> 
> Raul Gutierrez Segales wrote:
>     what about things like:
>     
>     https://github.com/apache/zookeeper/blob/trunk/src/java/main/org/apache/zookeeper/cli/CliCommand.java
>     
>     i see:
>     
>     ```
>     import org.apache.commons...
>     ```
>     
>     there.
> 
> Raul Gutierrez Segales wrote:
>     just asking to learn what the policy is, i am totally fine with not using it for this simple case (but if we can, sure it makes it nicer).

I hope you are asking about the way we have used the commons-cli.jar ?

Its for ZK shell commands - this simplifies the parsing logic of command line options.
Here first define the 'options' supported in my command. Now when user invokes the command, will just pass the 'options' and 'args' to the parser, rest he will take care for me.
Also, there are many other useful apis. Please have a look at CreateCommand.java

I've seen many apache projects has adopted this approach.


- Rakesh


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


On March 12, 2014, 4:17 a.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19089/
> -----------------------------------------------------------
> 
> (Updated March 12, 2014, 4:17 a.m.)
> 
> 
> Review request for zookeeper, fpj, michim, and Raul Gutierrez Segales.
> 
> 
> Bugs: ZOOKEEPER-1878
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1878
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> See ZOOKEEPER-1878
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1566210 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java 1566210 
> 
> Diff: https://reviews.apache.org/r/19089/diff/
> 
> 
> Testing
> -------
> 
> Test included
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 19089: Inconsistent behavior in autocreation of dataDir and dataLogDir

Posted by Rakesh R <ra...@huawei.com>.

> On March 12, 2014, 7:27 p.m., Camille Fournier wrote:
> > ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java, line 82
> > <https://reviews.apache.org/r/19089/diff/1/?file=517046#file517046line82>
> >
> >     Looks like the apache commons dep is only for the releaseaudit workflow, not for the project in general
> 
> Raul Gutierrez Segales wrote:
>     what about things like:
>     
>     https://github.com/apache/zookeeper/blob/trunk/src/java/main/org/apache/zookeeper/cli/CliCommand.java
>     
>     i see:
>     
>     ```
>     import org.apache.commons...
>     ```
>     
>     there.
> 
> Raul Gutierrez Segales wrote:
>     just asking to learn what the policy is, i am totally fine with not using it for this simple case (but if we can, sure it makes it nicer).
> 
> Rakesh R wrote:
>     I hope you are asking about the way we have used the commons-cli.jar ?
>     
>     Its for ZK shell commands - this simplifies the parsing logic of command line options.
>     Here first define the 'options' supported in my command. Now when user invokes the command, will just pass the 'options' and 'args' to the parser, rest he will take care for me.
>     Also, there are many other useful apis. Please have a look at CreateCommand.java
>     
>     I've seen many apache projects has adopted this approach.
> 
> Raul Gutierrez Segales wrote:
>     Oh, no no - I was asking why it was okey to use the apache commons stuff for that case (for the CLI parsing) and not from within test cases.  I just didn't grok what Camille meant that it could only be used for the releaseaudit workflow. Thanks for answering tho :-)

Hm :-)


- Rakesh


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


On March 12, 2014, 4:17 a.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19089/
> -----------------------------------------------------------
> 
> (Updated March 12, 2014, 4:17 a.m.)
> 
> 
> Review request for zookeeper, fpj, michim, and Raul Gutierrez Segales.
> 
> 
> Bugs: ZOOKEEPER-1878
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1878
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> See ZOOKEEPER-1878
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1566210 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java 1566210 
> 
> Diff: https://reviews.apache.org/r/19089/diff/
> 
> 
> Testing
> -------
> 
> Test included
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 19089: Inconsistent behavior in autocreation of dataDir and dataLogDir

Posted by Camille Fournier <sk...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19089/#review36966
-----------------------------------------------------------



./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java
<https://reviews.apache.org/r/19089/#comment68165>

    Looks like the apache commons dep is only for the releaseaudit workflow, not for the project in general


- Camille Fournier


On March 12, 2014, 4:17 a.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19089/
> -----------------------------------------------------------
> 
> (Updated March 12, 2014, 4:17 a.m.)
> 
> 
> Review request for zookeeper, fpj, michim, and Raul Gutierrez Segales.
> 
> 
> Bugs: ZOOKEEPER-1878
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1878
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> See ZOOKEEPER-1878
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1566210 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java 1566210 
> 
> Diff: https://reviews.apache.org/r/19089/diff/
> 
> 
> Testing
> -------
> 
> Test included
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 19089: Inconsistent behavior in autocreation of dataDir and dataLogDir

Posted by Rakesh R <ra...@huawei.com>.

> On March 12, 2014, 6:13 p.m., michim wrote:
> > ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java, line 68
> > <https://reviews.apache.org/r/19089/diff/1/?file=517046#file517046line68>
> >
> >     Can we remove this block? Doesn't the server create the data directory anyways?

I'll rename the 'autocreate' flag to 'preCreateDirs', so I hope it will avoid confusions. So we can verify tests with both the conditions with false/true. Is that meaningful?


- Rakesh


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


On March 12, 2014, 4:17 a.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19089/
> -----------------------------------------------------------
> 
> (Updated March 12, 2014, 4:17 a.m.)
> 
> 
> Review request for zookeeper, fpj, michim, and Raul Gutierrez Segales.
> 
> 
> Bugs: ZOOKEEPER-1878
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1878
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> See ZOOKEEPER-1878
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1566210 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java 1566210 
> 
> Diff: https://reviews.apache.org/r/19089/diff/
> 
> 
> Testing
> -------
> 
> Test included
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 19089: Inconsistent behavior in autocreation of dataDir and dataLogDir

Posted by mi...@cs.stanford.edu.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19089/#review36945
-----------------------------------------------------------



./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java
<https://reviews.apache.org/r/19089/#comment68132>

    Can we remove this block? Doesn't the server create the data directory anyways?


- michim


On March 12, 2014, 4:17 a.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19089/
> -----------------------------------------------------------
> 
> (Updated March 12, 2014, 4:17 a.m.)
> 
> 
> Review request for zookeeper, fpj, michim, and Raul Gutierrez Segales.
> 
> 
> Bugs: ZOOKEEPER-1878
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1878
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> See ZOOKEEPER-1878
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1566210 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java 1566210 
> 
> Diff: https://reviews.apache.org/r/19089/diff/
> 
> 
> Testing
> -------
> 
> Test included
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 19089: Inconsistent behavior in autocreation of dataDir and dataLogDir

Posted by Rakesh R <ra...@huawei.com>.

> On March 12, 2014, 5:43 p.m., Raul Gutierrez Segales wrote:
> > ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java, line 67
> > <https://reviews.apache.org/r/19089/diff/1/?file=517046#file517046line67>
> >
> >     nit: coding style (if (autocreate) {)

ok, I'll modify this.


> On March 12, 2014, 5:43 p.m., Raul Gutierrez Segales wrote:
> > ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java, line 72
> > <https://reviews.apache.org/r/19089/diff/1/?file=517046#file517046line72>
> >
> >     nit: coding style (spaces around else)

ok, I'll modify this.


> On March 12, 2014, 5:43 p.m., Raul Gutierrez Segales wrote:
> > ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java, line 82
> > <https://reviews.apache.org/r/19089/diff/1/?file=517046#file517046line82>
> >
> >     we already depend on Apache Commons, so you could use separatorsToSystem():
> >     
> >     http://people.apache.org/~jochen/commons-io/site/apidocs/org/apache/commons/io/FilenameUtils.html#separatorsToSystem%28java.lang.String%29

Hi Raul, Please see Camille's comment.


- Rakesh


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


On March 12, 2014, 4:17 a.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19089/
> -----------------------------------------------------------
> 
> (Updated March 12, 2014, 4:17 a.m.)
> 
> 
> Review request for zookeeper, fpj, michim, and Raul Gutierrez Segales.
> 
> 
> Bugs: ZOOKEEPER-1878
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1878
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> See ZOOKEEPER-1878
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1566210 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java 1566210 
> 
> Diff: https://reviews.apache.org/r/19089/diff/
> 
> 
> Testing
> -------
> 
> Test included
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 19089: Inconsistent behavior in autocreation of dataDir and dataLogDir

Posted by Rakesh R <ra...@huawei.com>.

> On March 12, 2014, 5:43 p.m., Raul Gutierrez Segales wrote:
> > ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java, line 274
> > <https://reviews.apache.org/r/19089/diff/1/?file=517045#file517045line274>
> >
> >     hmm, so at what point do we check if this dir exists then?

There is a check already present inside the constructor of FileTxnSnapLog.
 
FileTxnSnapLog(File dataDir, File snapDir){
    //....
    if (!this.snapDir.exists()) {
}

Will this be OK?


> On March 12, 2014, 5:43 p.m., Raul Gutierrez Segales wrote:
> > ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java, line 73
> > <https://reviews.apache.org/r/19089/diff/1/?file=517046#file517046line73>
> >
> >     no need to create this dir?

I'll rename the 'autocreate' flag to 'preCreateDirs', so I hope it will avoid confusions. Here the test expectation is, it sets "zookeeper.datadir.autocreate" to true and won't precreate the dataDir & dataLogDir, then zk server should create it and shouldn't throw exception. 


- Rakesh


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


On March 12, 2014, 4:17 a.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19089/
> -----------------------------------------------------------
> 
> (Updated March 12, 2014, 4:17 a.m.)
> 
> 
> Review request for zookeeper, fpj, michim, and Raul Gutierrez Segales.
> 
> 
> Bugs: ZOOKEEPER-1878
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1878
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> See ZOOKEEPER-1878
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1566210 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java 1566210 
> 
> Diff: https://reviews.apache.org/r/19089/diff/
> 
> 
> Testing
> -------
> 
> Test included
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 19089: Inconsistent behavior in autocreation of dataDir and dataLogDir

Posted by Raul Gutierrez Segales <rg...@itevenworks.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19089/#review36935
-----------------------------------------------------------



./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
<https://reviews.apache.org/r/19089/#comment68122>

    hmm, so at what point do we check if this dir exists then?



./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java
<https://reviews.apache.org/r/19089/#comment68120>

    nit: coding style (if (autocreate) {)



./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java
<https://reviews.apache.org/r/19089/#comment68121>

    nit: coding style (spaces around else)



./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java
<https://reviews.apache.org/r/19089/#comment68123>

    no need to create this dir?



./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java
<https://reviews.apache.org/r/19089/#comment68124>

    we already depend on Apache Commons, so you could use separatorsToSystem():
    
    http://people.apache.org/~jochen/commons-io/site/apidocs/org/apache/commons/io/FilenameUtils.html#separatorsToSystem%28java.lang.String%29


- Raul Gutierrez Segales


On March 12, 2014, 4:17 a.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19089/
> -----------------------------------------------------------
> 
> (Updated March 12, 2014, 4:17 a.m.)
> 
> 
> Review request for zookeeper, fpj, michim, and Raul Gutierrez Segales.
> 
> 
> Bugs: ZOOKEEPER-1878
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1878
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> See ZOOKEEPER-1878
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1566210 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java 1566210 
> 
> Diff: https://reviews.apache.org/r/19089/diff/
> 
> 
> Testing
> -------
> 
> Test included
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 19089: Inconsistent behavior in autocreation of dataDir and dataLogDir

Posted by Raul Gutierrez Segales <rg...@itevenworks.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19089/#review37219
-----------------------------------------------------------

Ship it!


Ship It!

- Raul Gutierrez Segales


On March 14, 2014, 2:44 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19089/
> -----------------------------------------------------------
> 
> (Updated March 14, 2014, 2:44 p.m.)
> 
> 
> Review request for zookeeper, fpj, michim, and Raul Gutierrez Segales.
> 
> 
> Bugs: ZOOKEEPER-1878
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1878
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> See ZOOKEEPER-1878
> 
> 
> Diffs
> -----
> 
>   ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1577459 
>   ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java 1577459 
> 
> Diff: https://reviews.apache.org/r/19089/diff/
> 
> 
> Testing
> -------
> 
> Test included
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Re: Review Request 19089: Inconsistent behavior in autocreation of dataDir and dataLogDir

Posted by Rakesh R <ra...@huawei.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19089/
-----------------------------------------------------------

(Updated March 14, 2014, 2:44 p.m.)


Review request for zookeeper, fpj, michim, and Raul Gutierrez Segales.


Changes
-------

Thanks Raul, Camille, Michi for the review comments.
Updated reworked patch.


Bugs: ZOOKEEPER-1878
    https://issues.apache.org/jira/browse/ZOOKEEPER-1878


Repository: zookeeper


Description
-------

See ZOOKEEPER-1878


Diffs (updated)
-----

  ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1577459 
  ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java 1577459 

Diff: https://reviews.apache.org/r/19089/diff/


Testing
-------

Test included


Thanks,

Rakesh R