> On July 10, 2012, 12:19 a.m., Benjamin Hindman wrote:
> > src/tests/zookeeper_tests.cpp, line 369
> > <https://reviews.apache.org/r/5869/diff/1/?file=121061#file121061line369>
> >
> >     Newline.

fixed


> On July 10, 2012, 12:19 a.m., Benjamin Hindman wrote:
> > src/zookeeper/group.cpp, line 414
> > <https://reviews.apache.org/r/5869/diff/1/?file=121062#file121062line414>
> >
> >     Why ignore the ZNOAUTH? What's the case in which we get ZNOAUTH as a 
> > result but then can actually create the ZNODE below? Maybe this just needs 
> > some more comments?

Added comments.  Could separate another elseif and do a get to verify exists 
but we can't write, but the idea was its simplest to just continue on and find 
out in the new final write check.


> On July 10, 2012, 12:19 a.m., Benjamin Hindman wrote:
> > src/zookeeper/group.cpp, line 426
> > <https://reviews.apache.org/r/5869/diff/1/?file=121062#file121062line426>
> >
> >     Please wrap for 80 characters. ;)

oops - fixed


- John


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


On July 10, 2012, 1:15 a.m., John Sirois wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5869/
> -----------------------------------------------------------
> 
> (Updated July 10, 2012, 1:15 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Description
> -------
> 
> commit 9c8316e84ec048df8d866fb50ebdf78d0e337d66
> Author: jsirois <[email protected]>
> Date:   Mon Jul 9 17:45:30 2012 -0600
> 
>     Handle secured parent-dirs in Group
>     
>     Modify Group to work with znodes that have read-only parent nodes
>     and only require the Group znode leaf itself is writable.
> 
>  src/tests/zookeeper_tests.cpp | 40 ++++++++++++++++++++++++++++++++++++++++
>  src/zookeeper/group.cpp       | 24 +++++++++++++++++++++++-
>  2 files changed, 63 insertions(+), 1 deletion(-)
> 
> 
> This addresses bug MESOS-229.
>     https://issues.apache.org/jira/browse/MESOS-229
> 
> 
> Diffs
> -----
> 
>   src/tests/zookeeper_tests.cpp d318ce7 
>   src/zookeeper/group.cpp a4a8b5a 
> 
> Diff: https://reviews.apache.org/r/5869/diff/
> 
> 
> Testing
> -------
> 
> Before fix with new test case:
> 
> $ ./bin/mesos-tests.sh 
> --gtest_filter=ZooKeeperTest.GroupPathWithRestrictivePerms
> Source directory: /Users/jsirois/development/third_party/mesos
> Build directory: /Users/jsirois/development/third_party/mesos/build
> Note: Google Test filter = ZooKeeperTest.GroupPathWithRestrictivePerms-
> [==========] Running 1 test from 1 test case.
> [----------] Global test environment set-up.
> [----------] 1 test from ZooKeeperTest
> [ RUN      ] ZooKeeperTest.GroupPathWithRestrictivePerms
> /Users/jsirois/development/third_party/mesos/build/../src/tests/zookeeper_tests.cpp:405:
>  Failure
> Value of: membership.isFailed()
>   Actual: true
> Expected: false
> Failed to create '/read-only/writable' in ZooKeeper: not authenticated
> [  FAILED  ] ZooKeeperTest.GroupPathWithRestrictivePerms (6346 ms)
> [----------] 1 test from ZooKeeperTest (6346 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 1 test from 1 test case ran. (6516 ms total)
> [  PASSED  ] 0 tests.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] ZooKeeperTest.GroupPathWithRestrictivePerms
> 
>  1 FAILED TEST
>   YOU HAVE 6 DISABLED TESTS
> 
> After fix:
> $ ./bin/mesos-tests.sh 
> --gtest_filter=ZooKeeperTest.GroupPathWithRestrictivePerms
> Source directory: /Users/jsirois/development/third_party/mesos
> Build directory: /Users/jsirois/development/third_party/mesos/build
> Note: Google Test filter = ZooKeeperTest.GroupPathWithRestrictivePerms-
> [==========] Running 1 test from 1 test case.
> [----------] Global test environment set-up.
> [----------] 1 test from ZooKeeperTest
> [ RUN      ] ZooKeeperTest.GroupPathWithRestrictivePerms
> [       OK ] ZooKeeperTest.GroupPathWithRestrictivePerms (6295 ms)
> [----------] 1 test from ZooKeeperTest (6295 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 1 test from 1 test case ran. (6469 ms total)
> [  PASSED  ] 1 test.
> 
>   YOU HAVE 6 DISABLED TESTS
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Reply via email to