You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ben Mahler <be...@gmail.com> on 2012/10/30 02:23:59 UTC

Review Request: Created a CHECK_SOME macro.

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

Review request for mesos, Benjamin Hindman and Vinod Kone.


Description
-------

This adds consistency with what we've done with ASSERT_SOME, EXPECT_SOME.


Diffs
-----

  src/java/jni/org_apache_mesos_Log.cpp e9ffccf8912977ff89cc5cb1ea1369ea5ce25647 
  src/log/coordinator.cpp 31d48e901825b44de0b5e66394cd9020108b45b6 
  src/log/log.hpp db026dc1edcf0bfe059dcff4036a1278d9933c26 
  src/log/log.cpp fcc343ef0e7cab3152d49dd837ce4da0347c8cd8 
  src/log/main.cpp 69d9443702e2ddf3b868957fe8f2712346a677de 
  src/log/replica.cpp b39623e8a9cb44cf4889b4d3351e952227810e54 
  src/logging/logging.hpp 8e8a4f6f92644c4db2d0d59925a930b6f0f72961 
  src/master/main.cpp 74c955f262179d52408d6b0eaada4d7801bcc193 
  src/slave/main.cpp 70bb71e427aaa5e1ebb1ae132c9aad3ebe9be34c 
  src/slave/paths.hpp 98e7fd402919c50a26f69a2f1a1904cb877c5f43 
  src/slave/process_based_isolation_module.cpp 744832625d6d1898eb2b31b184f5d0c68408835d 
  src/slave/slave.cpp 0321bc516166aacfd261c48f1f4293622d18ae0e 
  src/slave/state.cpp 65d91b889b84152ed12c4f2dd623b537242258df 
  src/tests/flags.hpp add9e0a1ec8c73e6556f9da2019e409edda305b3 
  src/tests/script.cpp 47fe9d72fd73a3cf73e59b7a0ba4818690e1f130 
  src/tests/slave_state_tests.cpp 7d09507d37f41c77f32ec04c7ecfc9657f196a30 
  src/tests/utils.cpp deef0bba1374cf8a8591813877a6170974d6de94 
  src/zookeeper/group.cpp 876b2ce4e94bfe45902ef86c3143b68ff30029f0 
  third_party/libprocess/include/process/future.hpp 49767c57c45f2a4d5c293b1ce0dcef3e40ab2e9f 
  third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6 
  third_party/libprocess/include/stout/numify.hpp 9f49e80c4f4d366264a699b12f84fc114f2dfe6a 

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


Testing
-------

CentOS: make check


Thanks,

Ben Mahler


Re: Review Request: Created a CHECK_SOME macro.

Posted by Ben Mahler <be...@gmail.com>.

> On Nov. 2, 2012, 5:26 p.m., Vinod Kone wrote:
> > third_party/libprocess/include/stout/numify.hpp, lines 25-28
> > <https://reviews.apache.org/r/7772/diff/1/?file=182211#file182211line25>
> >
> >     ditto?

I think benh wants the stout library to be standalone, in that it shouldn't require glog, where CHECK is defined.
This is a perfect use case for the ABORT function I created in another review.


> On Nov. 2, 2012, 5:26 p.m., Vinod Kone wrote:
> > third_party/libprocess/include/process/http.hpp, lines 307-312
> > <https://reviews.apache.org/r/7772/diff/1/?file=182210#file182210line307>
> >
> >     CHECK(1 <= UCHAR_MAX) << ...blah... 
> >     
> >     is more clear?

Do we want to use glog in our libprocess/include files?
If so, I'll switch it, but I'm not seeing many instances of CHECK in these includes, so I hesitated.


- Ben


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


On Oct. 30, 2012, 1:23 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7772/
> -----------------------------------------------------------
> 
> (Updated Oct. 30, 2012, 1:23 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This adds consistency with what we've done with ASSERT_SOME, EXPECT_SOME.
> 
> 
> Diffs
> -----
> 
>   src/java/jni/org_apache_mesos_Log.cpp e9ffccf8912977ff89cc5cb1ea1369ea5ce25647 
>   src/log/coordinator.cpp 31d48e901825b44de0b5e66394cd9020108b45b6 
>   src/log/log.hpp db026dc1edcf0bfe059dcff4036a1278d9933c26 
>   src/log/log.cpp fcc343ef0e7cab3152d49dd837ce4da0347c8cd8 
>   src/log/main.cpp 69d9443702e2ddf3b868957fe8f2712346a677de 
>   src/log/replica.cpp b39623e8a9cb44cf4889b4d3351e952227810e54 
>   src/logging/logging.hpp 8e8a4f6f92644c4db2d0d59925a930b6f0f72961 
>   src/master/main.cpp 74c955f262179d52408d6b0eaada4d7801bcc193 
>   src/slave/main.cpp 70bb71e427aaa5e1ebb1ae132c9aad3ebe9be34c 
>   src/slave/paths.hpp 98e7fd402919c50a26f69a2f1a1904cb877c5f43 
>   src/slave/process_based_isolation_module.cpp 744832625d6d1898eb2b31b184f5d0c68408835d 
>   src/slave/slave.cpp 0321bc516166aacfd261c48f1f4293622d18ae0e 
>   src/slave/state.cpp 65d91b889b84152ed12c4f2dd623b537242258df 
>   src/tests/flags.hpp add9e0a1ec8c73e6556f9da2019e409edda305b3 
>   src/tests/script.cpp 47fe9d72fd73a3cf73e59b7a0ba4818690e1f130 
>   src/tests/slave_state_tests.cpp 7d09507d37f41c77f32ec04c7ecfc9657f196a30 
>   src/tests/utils.cpp deef0bba1374cf8a8591813877a6170974d6de94 
>   src/zookeeper/group.cpp 876b2ce4e94bfe45902ef86c3143b68ff30029f0 
>   third_party/libprocess/include/process/future.hpp 49767c57c45f2a4d5c293b1ce0dcef3e40ab2e9f 
>   third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6 
>   third_party/libprocess/include/stout/numify.hpp 9f49e80c4f4d366264a699b12f84fc114f2dfe6a 
> 
> Diff: https://reviews.apache.org/r/7772/diff/
> 
> 
> Testing
> -------
> 
> CentOS: make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: Created a CHECK_SOME macro.

Posted by Vinod Kone <vi...@gmail.com>.

> On Nov. 2, 2012, 5:26 p.m., Vinod Kone wrote:
> > third_party/libprocess/include/process/http.hpp, lines 307-312
> > <https://reviews.apache.org/r/7772/diff/1/?file=182210#file182210line307>
> >
> >     CHECK(1 <= UCHAR_MAX) << ...blah... 
> >     
> >     is more clear?
> 
> Ben Mahler wrote:
>     Do we want to use glog in our libprocess/include files?
>     If so, I'll switch it, but I'm not seeing many instances of CHECK in these includes, so I hesitated.

aha. i see. sgtm then


- Vinod


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


On Oct. 30, 2012, 1:23 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7772/
> -----------------------------------------------------------
> 
> (Updated Oct. 30, 2012, 1:23 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This adds consistency with what we've done with ASSERT_SOME, EXPECT_SOME.
> 
> 
> Diffs
> -----
> 
>   src/java/jni/org_apache_mesos_Log.cpp e9ffccf8912977ff89cc5cb1ea1369ea5ce25647 
>   src/log/coordinator.cpp 31d48e901825b44de0b5e66394cd9020108b45b6 
>   src/log/log.hpp db026dc1edcf0bfe059dcff4036a1278d9933c26 
>   src/log/log.cpp fcc343ef0e7cab3152d49dd837ce4da0347c8cd8 
>   src/log/main.cpp 69d9443702e2ddf3b868957fe8f2712346a677de 
>   src/log/replica.cpp b39623e8a9cb44cf4889b4d3351e952227810e54 
>   src/logging/logging.hpp 8e8a4f6f92644c4db2d0d59925a930b6f0f72961 
>   src/master/main.cpp 74c955f262179d52408d6b0eaada4d7801bcc193 
>   src/slave/main.cpp 70bb71e427aaa5e1ebb1ae132c9aad3ebe9be34c 
>   src/slave/paths.hpp 98e7fd402919c50a26f69a2f1a1904cb877c5f43 
>   src/slave/process_based_isolation_module.cpp 744832625d6d1898eb2b31b184f5d0c68408835d 
>   src/slave/slave.cpp 0321bc516166aacfd261c48f1f4293622d18ae0e 
>   src/slave/state.cpp 65d91b889b84152ed12c4f2dd623b537242258df 
>   src/tests/flags.hpp add9e0a1ec8c73e6556f9da2019e409edda305b3 
>   src/tests/script.cpp 47fe9d72fd73a3cf73e59b7a0ba4818690e1f130 
>   src/tests/slave_state_tests.cpp 7d09507d37f41c77f32ec04c7ecfc9657f196a30 
>   src/tests/utils.cpp deef0bba1374cf8a8591813877a6170974d6de94 
>   src/zookeeper/group.cpp 876b2ce4e94bfe45902ef86c3143b68ff30029f0 
>   third_party/libprocess/include/process/future.hpp 49767c57c45f2a4d5c293b1ce0dcef3e40ab2e9f 
>   third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6 
>   third_party/libprocess/include/stout/numify.hpp 9f49e80c4f4d366264a699b12f84fc114f2dfe6a 
> 
> Diff: https://reviews.apache.org/r/7772/diff/
> 
> 
> Testing
> -------
> 
> CentOS: make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: Created a CHECK_SOME macro.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7772/#review13047
-----------------------------------------------------------



third_party/libprocess/include/process/http.hpp
<https://reviews.apache.org/r/7772/#comment28083>

    CHECK(1 <= UCHAR_MAX) << ...blah... 
    
    is more clear?



third_party/libprocess/include/stout/numify.hpp
<https://reviews.apache.org/r/7772/#comment28084>

    ditto?


- Vinod Kone


On Oct. 30, 2012, 1:23 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7772/
> -----------------------------------------------------------
> 
> (Updated Oct. 30, 2012, 1:23 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This adds consistency with what we've done with ASSERT_SOME, EXPECT_SOME.
> 
> 
> Diffs
> -----
> 
>   src/java/jni/org_apache_mesos_Log.cpp e9ffccf8912977ff89cc5cb1ea1369ea5ce25647 
>   src/log/coordinator.cpp 31d48e901825b44de0b5e66394cd9020108b45b6 
>   src/log/log.hpp db026dc1edcf0bfe059dcff4036a1278d9933c26 
>   src/log/log.cpp fcc343ef0e7cab3152d49dd837ce4da0347c8cd8 
>   src/log/main.cpp 69d9443702e2ddf3b868957fe8f2712346a677de 
>   src/log/replica.cpp b39623e8a9cb44cf4889b4d3351e952227810e54 
>   src/logging/logging.hpp 8e8a4f6f92644c4db2d0d59925a930b6f0f72961 
>   src/master/main.cpp 74c955f262179d52408d6b0eaada4d7801bcc193 
>   src/slave/main.cpp 70bb71e427aaa5e1ebb1ae132c9aad3ebe9be34c 
>   src/slave/paths.hpp 98e7fd402919c50a26f69a2f1a1904cb877c5f43 
>   src/slave/process_based_isolation_module.cpp 744832625d6d1898eb2b31b184f5d0c68408835d 
>   src/slave/slave.cpp 0321bc516166aacfd261c48f1f4293622d18ae0e 
>   src/slave/state.cpp 65d91b889b84152ed12c4f2dd623b537242258df 
>   src/tests/flags.hpp add9e0a1ec8c73e6556f9da2019e409edda305b3 
>   src/tests/script.cpp 47fe9d72fd73a3cf73e59b7a0ba4818690e1f130 
>   src/tests/slave_state_tests.cpp 7d09507d37f41c77f32ec04c7ecfc9657f196a30 
>   src/tests/utils.cpp deef0bba1374cf8a8591813877a6170974d6de94 
>   src/zookeeper/group.cpp 876b2ce4e94bfe45902ef86c3143b68ff30029f0 
>   third_party/libprocess/include/process/future.hpp 49767c57c45f2a4d5c293b1ce0dcef3e40ab2e9f 
>   third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6 
>   third_party/libprocess/include/stout/numify.hpp 9f49e80c4f4d366264a699b12f84fc114f2dfe6a 
> 
> Diff: https://reviews.apache.org/r/7772/diff/
> 
> 
> Testing
> -------
> 
> CentOS: make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: Created a CHECK_SOME macro.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7772/#review13110
-----------------------------------------------------------



src/logging/logging.hpp
<https://reviews.apache.org/r/7772/#comment28243>

    The hope here was that CHECK_SOME will always print out the error (so you didn't always have to do it yourself) kind of like PCHECK (i.e., after what ever string you put down it would append a ": " and then the error).


- Benjamin Hindman


On Oct. 30, 2012, 1:23 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7772/
> -----------------------------------------------------------
> 
> (Updated Oct. 30, 2012, 1:23 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This adds consistency with what we've done with ASSERT_SOME, EXPECT_SOME.
> 
> 
> Diffs
> -----
> 
>   src/java/jni/org_apache_mesos_Log.cpp e9ffccf8912977ff89cc5cb1ea1369ea5ce25647 
>   src/log/coordinator.cpp 31d48e901825b44de0b5e66394cd9020108b45b6 
>   src/log/log.hpp db026dc1edcf0bfe059dcff4036a1278d9933c26 
>   src/log/log.cpp fcc343ef0e7cab3152d49dd837ce4da0347c8cd8 
>   src/log/main.cpp 69d9443702e2ddf3b868957fe8f2712346a677de 
>   src/log/replica.cpp b39623e8a9cb44cf4889b4d3351e952227810e54 
>   src/logging/logging.hpp 8e8a4f6f92644c4db2d0d59925a930b6f0f72961 
>   src/master/main.cpp 74c955f262179d52408d6b0eaada4d7801bcc193 
>   src/slave/main.cpp 70bb71e427aaa5e1ebb1ae132c9aad3ebe9be34c 
>   src/slave/paths.hpp 98e7fd402919c50a26f69a2f1a1904cb877c5f43 
>   src/slave/process_based_isolation_module.cpp 744832625d6d1898eb2b31b184f5d0c68408835d 
>   src/slave/slave.cpp 0321bc516166aacfd261c48f1f4293622d18ae0e 
>   src/slave/state.cpp 65d91b889b84152ed12c4f2dd623b537242258df 
>   src/tests/flags.hpp add9e0a1ec8c73e6556f9da2019e409edda305b3 
>   src/tests/script.cpp 47fe9d72fd73a3cf73e59b7a0ba4818690e1f130 
>   src/tests/slave_state_tests.cpp 7d09507d37f41c77f32ec04c7ecfc9657f196a30 
>   src/tests/utils.cpp deef0bba1374cf8a8591813877a6170974d6de94 
>   src/zookeeper/group.cpp 876b2ce4e94bfe45902ef86c3143b68ff30029f0 
>   third_party/libprocess/include/process/future.hpp 49767c57c45f2a4d5c293b1ce0dcef3e40ab2e9f 
>   third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6 
>   third_party/libprocess/include/stout/numify.hpp 9f49e80c4f4d366264a699b12f84fc114f2dfe6a 
> 
> Diff: https://reviews.apache.org/r/7772/diff/
> 
> 
> Testing
> -------
> 
> CentOS: make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: Created a CHECK_SOME macro.

Posted by Benjamin Hindman <be...@berkeley.edu>.

> On Nov. 15, 2012, 4:20 p.m., Benjamin Hindman wrote:
> > third_party/libprocess/include/process/http.hpp, line 307
> > <https://reviews.apache.org/r/7772/diff/2/?file=186085#file186085line307>
> >
> >     I thought you check the digits above? (Hence the CHECK here).
> 
> Ben Mahler wrote:
>     Yeah I just changed from CHECK to abort, since you mentioned not really wanting to introduce dependencies on glog in the libprocess includes.

Gotcha, thanks.


> On Nov. 15, 2012, 4:20 p.m., Benjamin Hindman wrote:
> > third_party/libprocess/include/stout/numify.hpp, line 24
> > <https://reviews.apache.org/r/7772/diff/2/?file=186086#file186086line24>
> >
> >     In the meantime, why don't you like this CHECK? With the check we get a stack trace, which in this case will probably help us track down a bug (this doesn't seem like an instance where we want to have better output for our users, this is a legitimate crash).
> 
> Ben Mahler wrote:
>     But numify.hpp lives in stout.. which you didn't want depending on glog right?
>     
>     Also, as an aside, I plan to refactor strings::format to not return a Try.

Indeed, thanks!


- Benjamin


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


On Nov. 15, 2012, 7:39 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7772/
> -----------------------------------------------------------
> 
> (Updated Nov. 15, 2012, 7:39 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This adds consistency with what we've done with ASSERT_SOME, EXPECT_SOME.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am b2d1edf140797c7150cb4644d323296965c4f000 
>   src/java/jni/org_apache_mesos_Log.cpp e9ffccf8912977ff89cc5cb1ea1369ea5ce25647 
>   src/log/coordinator.cpp 31d48e901825b44de0b5e66394cd9020108b45b6 
>   src/log/log.hpp db026dc1edcf0bfe059dcff4036a1278d9933c26 
>   src/log/log.cpp fcc343ef0e7cab3152d49dd837ce4da0347c8cd8 
>   src/log/main.cpp 69d9443702e2ddf3b868957fe8f2712346a677de 
>   src/log/replica.cpp b39623e8a9cb44cf4889b4d3351e952227810e54 
>   src/logging/check_some.hpp PRE-CREATION 
>   src/logging/logging.hpp 8e8a4f6f92644c4db2d0d59925a930b6f0f72961 
>   src/master/main.cpp 9e00e738df26af07f5edfbc79a8a2812c57db287 
>   src/slave/main.cpp 70bb71e427aaa5e1ebb1ae132c9aad3ebe9be34c 
>   src/slave/paths.hpp 98e7fd402919c50a26f69a2f1a1904cb877c5f43 
>   src/slave/process_based_isolation_module.cpp 16fd584e78db2c517d828f2576ab8a38c5ce57ad 
>   src/slave/slave.cpp 7deb4574943aae4cfc5da5d6b3f600042686975f 
>   src/slave/state.cpp 65d91b889b84152ed12c4f2dd623b537242258df 
>   src/tests/flags.hpp add9e0a1ec8c73e6556f9da2019e409edda305b3 
>   src/tests/script.cpp 783b369db44ab73132cc0b7d22a3c1f4d1928fda 
>   src/tests/slave_state_tests.cpp 7d09507d37f41c77f32ec04c7ecfc9657f196a30 
>   src/tests/utils.cpp deef0bba1374cf8a8591813877a6170974d6de94 
>   src/zookeeper/group.cpp 876b2ce4e94bfe45902ef86c3143b68ff30029f0 
>   third_party/libprocess/include/process/future.hpp 49767c57c45f2a4d5c293b1ce0dcef3e40ab2e9f 
>   third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6 
>   third_party/libprocess/include/stout/numify.hpp 9f49e80c4f4d366264a699b12f84fc114f2dfe6a 
> 
> Diff: https://reviews.apache.org/r/7772/diff/
> 
> 
> Testing
> -------
> 
> CentOS: make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: Created a CHECK_SOME macro.

Posted by Ben Mahler <be...@gmail.com>.

> On Nov. 15, 2012, 4:20 p.m., Benjamin Hindman wrote:
> > src/log/replica.cpp, line 1011
> > <https://reviews.apache.org/r/7772/diff/2/?file=186071#file186071line1011>
> >
> >     You're double printing the error (hence the desire for CHECK_SOME).

Ah, leftovers from the first version. Fixed.


> On Nov. 15, 2012, 4:20 p.m., Benjamin Hindman wrote:
> > third_party/libprocess/include/stout/numify.hpp, line 24
> > <https://reviews.apache.org/r/7772/diff/2/?file=186086#file186086line24>
> >
> >     In the meantime, why don't you like this CHECK? With the check we get a stack trace, which in this case will probably help us track down a bug (this doesn't seem like an instance where we want to have better output for our users, this is a legitimate crash).

But numify.hpp lives in stout.. which you didn't want depending on glog right?

Also, as an aside, I plan to refactor strings::format to not return a Try.


> On Nov. 15, 2012, 4:20 p.m., Benjamin Hindman wrote:
> > third_party/libprocess/include/process/http.hpp, line 307
> > <https://reviews.apache.org/r/7772/diff/2/?file=186085#file186085line307>
> >
> >     I thought you check the digits above? (Hence the CHECK here).

Yeah I just changed from CHECK to abort, since you mentioned not really wanting to introduce dependencies on glog in the libprocess includes.


> On Nov. 15, 2012, 4:20 p.m., Benjamin Hindman wrote:
> > third_party/libprocess/include/process/future.hpp, line 532
> > <https://reviews.apache.org/r/7772/diff/2/?file=186084#file186084line532>
> >
> >     This CHECK is for us, the library writers, so that we make sure that we always have transitioned a future out of pending after await. The errors below and the abort is because a programmer decided to call get and block, and while we should always transition out of pending after await the future might have failed and so we can't return any value.

Ok reverted this bit.


- Ben


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


On Nov. 6, 2012, 11:55 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7772/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2012, 11:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This adds consistency with what we've done with ASSERT_SOME, EXPECT_SOME.
> 
> 
> Diffs
> -----
> 
>   src/java/jni/org_apache_mesos_Log.cpp e9ffccf8912977ff89cc5cb1ea1369ea5ce25647 
>   src/log/coordinator.cpp 31d48e901825b44de0b5e66394cd9020108b45b6 
>   src/log/log.hpp db026dc1edcf0bfe059dcff4036a1278d9933c26 
>   src/log/log.cpp fcc343ef0e7cab3152d49dd837ce4da0347c8cd8 
>   src/log/main.cpp 69d9443702e2ddf3b868957fe8f2712346a677de 
>   src/log/replica.cpp b39623e8a9cb44cf4889b4d3351e952227810e54 
>   src/logging/logging.hpp 8e8a4f6f92644c4db2d0d59925a930b6f0f72961 
>   src/master/main.cpp 74c955f262179d52408d6b0eaada4d7801bcc193 
>   src/slave/main.cpp 70bb71e427aaa5e1ebb1ae132c9aad3ebe9be34c 
>   src/slave/paths.hpp 98e7fd402919c50a26f69a2f1a1904cb877c5f43 
>   src/slave/process_based_isolation_module.cpp b0b6a81c93acc68d1f4acbdda5ab2f9f96b5fb5a 
>   src/slave/slave.cpp 2bd2dbce538a6108dd9fe607829cfbdab33e0778 
>   src/slave/state.cpp 65d91b889b84152ed12c4f2dd623b537242258df 
>   src/tests/flags.hpp add9e0a1ec8c73e6556f9da2019e409edda305b3 
>   src/tests/script.cpp 783b369db44ab73132cc0b7d22a3c1f4d1928fda 
>   src/tests/slave_state_tests.cpp 7d09507d37f41c77f32ec04c7ecfc9657f196a30 
>   src/tests/utils.cpp deef0bba1374cf8a8591813877a6170974d6de94 
>   src/zookeeper/group.cpp 876b2ce4e94bfe45902ef86c3143b68ff30029f0 
>   third_party/libprocess/include/process/future.hpp 49767c57c45f2a4d5c293b1ce0dcef3e40ab2e9f 
>   third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6 
>   third_party/libprocess/include/stout/numify.hpp 9f49e80c4f4d366264a699b12f84fc114f2dfe6a 
> 
> Diff: https://reviews.apache.org/r/7772/diff/
> 
> 
> Testing
> -------
> 
> CentOS: make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: Created a CHECK_SOME macro.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7772/#review13468
-----------------------------------------------------------



src/log/replica.cpp
<https://reviews.apache.org/r/7772/#comment28854>

    You're double printing the error (hence the desire for CHECK_SOME).



src/master/main.cpp
<https://reviews.apache.org/r/7772/#comment28853>

    You're double printing the error (hence the desire for CHECK_SOME).



src/slave/main.cpp
<https://reviews.apache.org/r/7772/#comment28855>

    Ditto.



src/slave/paths.hpp
<https://reviews.apache.org/r/7772/#comment28856>

    Ditto.



src/slave/paths.hpp
<https://reviews.apache.org/r/7772/#comment28857>

    Ditto.



src/slave/process_based_isolation_module.cpp
<https://reviews.apache.org/r/7772/#comment28858>

    Ditto.



src/slave/process_based_isolation_module.cpp
<https://reviews.apache.org/r/7772/#comment28859>

    Ditto.



src/slave/state.cpp
<https://reviews.apache.org/r/7772/#comment28860>

    Ditto.



src/slave/state.cpp
<https://reviews.apache.org/r/7772/#comment28861>

    Ditto.



src/slave/state.cpp
<https://reviews.apache.org/r/7772/#comment28862>

    Dittoish (need to kill 'strerror(errno)').



src/slave/state.cpp
<https://reviews.apache.org/r/7772/#comment28863>

    Ditto.



src/slave/state.cpp
<https://reviews.apache.org/r/7772/#comment28864>

    Dittoish again.



src/tests/flags.hpp
<https://reviews.apache.org/r/7772/#comment28865>

    Ditto.



src/tests/flags.hpp
<https://reviews.apache.org/r/7772/#comment28866>

    Ditto.



src/tests/script.cpp
<https://reviews.apache.org/r/7772/#comment28867>

    Ditto.



src/tests/slave_state_tests.cpp
<https://reviews.apache.org/r/7772/#comment28868>

    Ditto.



src/tests/utils.cpp
<https://reviews.apache.org/r/7772/#comment28869>

    Ditto.



src/zookeeper/group.cpp
<https://reviews.apache.org/r/7772/#comment28870>

    Ditto.



src/zookeeper/group.cpp
<https://reviews.apache.org/r/7772/#comment28871>

    Ditto.



src/zookeeper/group.cpp
<https://reviews.apache.org/r/7772/#comment28872>

    Ditto.



third_party/libprocess/include/process/future.hpp
<https://reviews.apache.org/r/7772/#comment28848>

    This CHECK is for us, the library writers, so that we make sure that we always have transitioned a future out of pending after await. The errors below and the abort is because a programmer decided to call get and block, and while we should always transition out of pending after await the future might have failed and so we can't return any value.



third_party/libprocess/include/process/http.hpp
<https://reviews.apache.org/r/7772/#comment28849>

    I thought you check the digits above? (Hence the CHECK here).



third_party/libprocess/include/stout/numify.hpp
<https://reviews.apache.org/r/7772/#comment28852>

    In the meantime, why don't you like this CHECK? With the check we get a stack trace, which in this case will probably help us track down a bug (this doesn't seem like an instance where we want to have better output for our users, this is a legitimate crash).


- Benjamin Hindman


On Nov. 6, 2012, 11:55 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7772/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2012, 11:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This adds consistency with what we've done with ASSERT_SOME, EXPECT_SOME.
> 
> 
> Diffs
> -----
> 
>   src/java/jni/org_apache_mesos_Log.cpp e9ffccf8912977ff89cc5cb1ea1369ea5ce25647 
>   src/log/coordinator.cpp 31d48e901825b44de0b5e66394cd9020108b45b6 
>   src/log/log.hpp db026dc1edcf0bfe059dcff4036a1278d9933c26 
>   src/log/log.cpp fcc343ef0e7cab3152d49dd837ce4da0347c8cd8 
>   src/log/main.cpp 69d9443702e2ddf3b868957fe8f2712346a677de 
>   src/log/replica.cpp b39623e8a9cb44cf4889b4d3351e952227810e54 
>   src/logging/logging.hpp 8e8a4f6f92644c4db2d0d59925a930b6f0f72961 
>   src/master/main.cpp 74c955f262179d52408d6b0eaada4d7801bcc193 
>   src/slave/main.cpp 70bb71e427aaa5e1ebb1ae132c9aad3ebe9be34c 
>   src/slave/paths.hpp 98e7fd402919c50a26f69a2f1a1904cb877c5f43 
>   src/slave/process_based_isolation_module.cpp b0b6a81c93acc68d1f4acbdda5ab2f9f96b5fb5a 
>   src/slave/slave.cpp 2bd2dbce538a6108dd9fe607829cfbdab33e0778 
>   src/slave/state.cpp 65d91b889b84152ed12c4f2dd623b537242258df 
>   src/tests/flags.hpp add9e0a1ec8c73e6556f9da2019e409edda305b3 
>   src/tests/script.cpp 783b369db44ab73132cc0b7d22a3c1f4d1928fda 
>   src/tests/slave_state_tests.cpp 7d09507d37f41c77f32ec04c7ecfc9657f196a30 
>   src/tests/utils.cpp deef0bba1374cf8a8591813877a6170974d6de94 
>   src/zookeeper/group.cpp 876b2ce4e94bfe45902ef86c3143b68ff30029f0 
>   third_party/libprocess/include/process/future.hpp 49767c57c45f2a4d5c293b1ce0dcef3e40ab2e9f 
>   third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6 
>   third_party/libprocess/include/stout/numify.hpp 9f49e80c4f4d366264a699b12f84fc114f2dfe6a 
> 
> Diff: https://reviews.apache.org/r/7772/diff/
> 
> 
> Testing
> -------
> 
> CentOS: make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: Created a CHECK_SOME macro.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7772/#review13467
-----------------------------------------------------------



src/logging/logging.hpp
<https://reviews.apache.org/r/7772/#comment28845>

    This is great but lets move this into logging/check_some.hpp to keep this file simpler but have this file include check_some.hpp. 


- Benjamin Hindman


On Nov. 6, 2012, 11:55 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7772/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2012, 11:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This adds consistency with what we've done with ASSERT_SOME, EXPECT_SOME.
> 
> 
> Diffs
> -----
> 
>   src/java/jni/org_apache_mesos_Log.cpp e9ffccf8912977ff89cc5cb1ea1369ea5ce25647 
>   src/log/coordinator.cpp 31d48e901825b44de0b5e66394cd9020108b45b6 
>   src/log/log.hpp db026dc1edcf0bfe059dcff4036a1278d9933c26 
>   src/log/log.cpp fcc343ef0e7cab3152d49dd837ce4da0347c8cd8 
>   src/log/main.cpp 69d9443702e2ddf3b868957fe8f2712346a677de 
>   src/log/replica.cpp b39623e8a9cb44cf4889b4d3351e952227810e54 
>   src/logging/logging.hpp 8e8a4f6f92644c4db2d0d59925a930b6f0f72961 
>   src/master/main.cpp 74c955f262179d52408d6b0eaada4d7801bcc193 
>   src/slave/main.cpp 70bb71e427aaa5e1ebb1ae132c9aad3ebe9be34c 
>   src/slave/paths.hpp 98e7fd402919c50a26f69a2f1a1904cb877c5f43 
>   src/slave/process_based_isolation_module.cpp b0b6a81c93acc68d1f4acbdda5ab2f9f96b5fb5a 
>   src/slave/slave.cpp 2bd2dbce538a6108dd9fe607829cfbdab33e0778 
>   src/slave/state.cpp 65d91b889b84152ed12c4f2dd623b537242258df 
>   src/tests/flags.hpp add9e0a1ec8c73e6556f9da2019e409edda305b3 
>   src/tests/script.cpp 783b369db44ab73132cc0b7d22a3c1f4d1928fda 
>   src/tests/slave_state_tests.cpp 7d09507d37f41c77f32ec04c7ecfc9657f196a30 
>   src/tests/utils.cpp deef0bba1374cf8a8591813877a6170974d6de94 
>   src/zookeeper/group.cpp 876b2ce4e94bfe45902ef86c3143b68ff30029f0 
>   third_party/libprocess/include/process/future.hpp 49767c57c45f2a4d5c293b1ce0dcef3e40ab2e9f 
>   third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6 
>   third_party/libprocess/include/stout/numify.hpp 9f49e80c4f4d366264a699b12f84fc114f2dfe6a 
> 
> Diff: https://reviews.apache.org/r/7772/diff/
> 
> 
> Testing
> -------
> 
> CentOS: make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: Created a CHECK_SOME macro.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7772/#review14009
-----------------------------------------------------------

Ship it!



src/logging/check_some.hpp
<https://reviews.apache.org/r/7772/#comment29980>

    s/"s/<s/g
    s/p"/p>/g


- Benjamin Hindman


On Nov. 15, 2012, 7:39 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7772/
> -----------------------------------------------------------
> 
> (Updated Nov. 15, 2012, 7:39 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This adds consistency with what we've done with ASSERT_SOME, EXPECT_SOME.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am b2d1edf140797c7150cb4644d323296965c4f000 
>   src/java/jni/org_apache_mesos_Log.cpp e9ffccf8912977ff89cc5cb1ea1369ea5ce25647 
>   src/log/coordinator.cpp 31d48e901825b44de0b5e66394cd9020108b45b6 
>   src/log/log.hpp db026dc1edcf0bfe059dcff4036a1278d9933c26 
>   src/log/log.cpp fcc343ef0e7cab3152d49dd837ce4da0347c8cd8 
>   src/log/main.cpp 69d9443702e2ddf3b868957fe8f2712346a677de 
>   src/log/replica.cpp b39623e8a9cb44cf4889b4d3351e952227810e54 
>   src/logging/check_some.hpp PRE-CREATION 
>   src/logging/logging.hpp 8e8a4f6f92644c4db2d0d59925a930b6f0f72961 
>   src/master/main.cpp 9e00e738df26af07f5edfbc79a8a2812c57db287 
>   src/slave/main.cpp 70bb71e427aaa5e1ebb1ae132c9aad3ebe9be34c 
>   src/slave/paths.hpp 98e7fd402919c50a26f69a2f1a1904cb877c5f43 
>   src/slave/process_based_isolation_module.cpp 16fd584e78db2c517d828f2576ab8a38c5ce57ad 
>   src/slave/slave.cpp 7deb4574943aae4cfc5da5d6b3f600042686975f 
>   src/slave/state.cpp 65d91b889b84152ed12c4f2dd623b537242258df 
>   src/tests/flags.hpp add9e0a1ec8c73e6556f9da2019e409edda305b3 
>   src/tests/script.cpp 783b369db44ab73132cc0b7d22a3c1f4d1928fda 
>   src/tests/slave_state_tests.cpp 7d09507d37f41c77f32ec04c7ecfc9657f196a30 
>   src/tests/utils.cpp deef0bba1374cf8a8591813877a6170974d6de94 
>   src/zookeeper/group.cpp 876b2ce4e94bfe45902ef86c3143b68ff30029f0 
>   third_party/libprocess/include/process/future.hpp 49767c57c45f2a4d5c293b1ce0dcef3e40ab2e9f 
>   third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6 
>   third_party/libprocess/include/stout/numify.hpp 9f49e80c4f4d366264a699b12f84fc114f2dfe6a 
> 
> Diff: https://reviews.apache.org/r/7772/diff/
> 
> 
> Testing
> -------
> 
> CentOS: make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: Created a CHECK_SOME macro.

Posted by Ben Mahler <be...@gmail.com>.

> On Dec. 4, 2012, 6:49 a.m., Benjamin Hindman wrote:
> > src/logging/check_some.hpp, lines 74-77
> > <https://reviews.apache.org/r/7772/diff/3/?file=190345#file190345line74>
> >
> >     Also, I've never read this part of the standard, but what makes this initializer list legal?

I'll change this to make it more clear, I've just done it this way in the past so this was out of habit.

See 12.6.2/7 of the spec if you're curious (p. 199):
http://www-d0.fnal.gov/~dladams/cxx_standard.pdf


- Ben


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


On Nov. 15, 2012, 7:39 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7772/
> -----------------------------------------------------------
> 
> (Updated Nov. 15, 2012, 7:39 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This adds consistency with what we've done with ASSERT_SOME, EXPECT_SOME.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am b2d1edf140797c7150cb4644d323296965c4f000 
>   src/java/jni/org_apache_mesos_Log.cpp e9ffccf8912977ff89cc5cb1ea1369ea5ce25647 
>   src/log/coordinator.cpp 31d48e901825b44de0b5e66394cd9020108b45b6 
>   src/log/log.hpp db026dc1edcf0bfe059dcff4036a1278d9933c26 
>   src/log/log.cpp fcc343ef0e7cab3152d49dd837ce4da0347c8cd8 
>   src/log/main.cpp 69d9443702e2ddf3b868957fe8f2712346a677de 
>   src/log/replica.cpp b39623e8a9cb44cf4889b4d3351e952227810e54 
>   src/logging/check_some.hpp PRE-CREATION 
>   src/logging/logging.hpp 8e8a4f6f92644c4db2d0d59925a930b6f0f72961 
>   src/master/main.cpp 9e00e738df26af07f5edfbc79a8a2812c57db287 
>   src/slave/main.cpp 70bb71e427aaa5e1ebb1ae132c9aad3ebe9be34c 
>   src/slave/paths.hpp 98e7fd402919c50a26f69a2f1a1904cb877c5f43 
>   src/slave/process_based_isolation_module.cpp 16fd584e78db2c517d828f2576ab8a38c5ce57ad 
>   src/slave/slave.cpp 7deb4574943aae4cfc5da5d6b3f600042686975f 
>   src/slave/state.cpp 65d91b889b84152ed12c4f2dd623b537242258df 
>   src/tests/flags.hpp add9e0a1ec8c73e6556f9da2019e409edda305b3 
>   src/tests/script.cpp 783b369db44ab73132cc0b7d22a3c1f4d1928fda 
>   src/tests/slave_state_tests.cpp 7d09507d37f41c77f32ec04c7ecfc9657f196a30 
>   src/tests/utils.cpp deef0bba1374cf8a8591813877a6170974d6de94 
>   src/zookeeper/group.cpp 876b2ce4e94bfe45902ef86c3143b68ff30029f0 
>   third_party/libprocess/include/process/future.hpp 49767c57c45f2a4d5c293b1ce0dcef3e40ab2e9f 
>   third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6 
>   third_party/libprocess/include/stout/numify.hpp 9f49e80c4f4d366264a699b12f84fc114f2dfe6a 
> 
> Diff: https://reviews.apache.org/r/7772/diff/
> 
> 
> Testing
> -------
> 
> CentOS: make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: Created a CHECK_SOME macro.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7772/#review14011
-----------------------------------------------------------



src/logging/check_some.hpp
<https://reviews.apache.org/r/7772/#comment29982>

    Also, I've never read this part of the standard, but what makes this initializer list legal?


- Benjamin Hindman


On Nov. 15, 2012, 7:39 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7772/
> -----------------------------------------------------------
> 
> (Updated Nov. 15, 2012, 7:39 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This adds consistency with what we've done with ASSERT_SOME, EXPECT_SOME.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am b2d1edf140797c7150cb4644d323296965c4f000 
>   src/java/jni/org_apache_mesos_Log.cpp e9ffccf8912977ff89cc5cb1ea1369ea5ce25647 
>   src/log/coordinator.cpp 31d48e901825b44de0b5e66394cd9020108b45b6 
>   src/log/log.hpp db026dc1edcf0bfe059dcff4036a1278d9933c26 
>   src/log/log.cpp fcc343ef0e7cab3152d49dd837ce4da0347c8cd8 
>   src/log/main.cpp 69d9443702e2ddf3b868957fe8f2712346a677de 
>   src/log/replica.cpp b39623e8a9cb44cf4889b4d3351e952227810e54 
>   src/logging/check_some.hpp PRE-CREATION 
>   src/logging/logging.hpp 8e8a4f6f92644c4db2d0d59925a930b6f0f72961 
>   src/master/main.cpp 9e00e738df26af07f5edfbc79a8a2812c57db287 
>   src/slave/main.cpp 70bb71e427aaa5e1ebb1ae132c9aad3ebe9be34c 
>   src/slave/paths.hpp 98e7fd402919c50a26f69a2f1a1904cb877c5f43 
>   src/slave/process_based_isolation_module.cpp 16fd584e78db2c517d828f2576ab8a38c5ce57ad 
>   src/slave/slave.cpp 7deb4574943aae4cfc5da5d6b3f600042686975f 
>   src/slave/state.cpp 65d91b889b84152ed12c4f2dd623b537242258df 
>   src/tests/flags.hpp add9e0a1ec8c73e6556f9da2019e409edda305b3 
>   src/tests/script.cpp 783b369db44ab73132cc0b7d22a3c1f4d1928fda 
>   src/tests/slave_state_tests.cpp 7d09507d37f41c77f32ec04c7ecfc9657f196a30 
>   src/tests/utils.cpp deef0bba1374cf8a8591813877a6170974d6de94 
>   src/zookeeper/group.cpp 876b2ce4e94bfe45902ef86c3143b68ff30029f0 
>   third_party/libprocess/include/process/future.hpp 49767c57c45f2a4d5c293b1ce0dcef3e40ab2e9f 
>   third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6 
>   third_party/libprocess/include/stout/numify.hpp 9f49e80c4f4d366264a699b12f84fc114f2dfe6a 
> 
> Diff: https://reviews.apache.org/r/7772/diff/
> 
> 
> Testing
> -------
> 
> CentOS: make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: Created a CHECK_SOME macro.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7772/#review14121
-----------------------------------------------------------

Ship it!


Ship It!

- Benjamin Hindman


On Dec. 6, 2012, 10 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7772/
> -----------------------------------------------------------
> 
> (Updated Dec. 6, 2012, 10 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This adds consistency with what we've done with ASSERT_SOME, EXPECT_SOME.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am b2d1edf140797c7150cb4644d323296965c4f000 
>   src/java/jni/org_apache_mesos_Log.cpp e9ffccf8912977ff89cc5cb1ea1369ea5ce25647 
>   src/log/coordinator.cpp 31d48e901825b44de0b5e66394cd9020108b45b6 
>   src/log/log.hpp db026dc1edcf0bfe059dcff4036a1278d9933c26 
>   src/log/log.cpp fcc343ef0e7cab3152d49dd837ce4da0347c8cd8 
>   src/log/main.cpp 69d9443702e2ddf3b868957fe8f2712346a677de 
>   src/log/replica.cpp b39623e8a9cb44cf4889b4d3351e952227810e54 
>   src/logging/check_some.hpp PRE-CREATION 
>   src/logging/logging.hpp 8e8a4f6f92644c4db2d0d59925a930b6f0f72961 
>   src/master/main.cpp 9e00e738df26af07f5edfbc79a8a2812c57db287 
>   src/slave/main.cpp 70bb71e427aaa5e1ebb1ae132c9aad3ebe9be34c 
>   src/slave/paths.hpp 98e7fd402919c50a26f69a2f1a1904cb877c5f43 
>   src/slave/process_based_isolation_module.cpp ede080069d814f410662f759806d1d5b260e8569 
>   src/slave/slave.cpp 28fd4c336d8ac658cf92811d20066a6cfdf5a95e 
>   src/slave/state.cpp 65d91b889b84152ed12c4f2dd623b537242258df 
>   src/tests/flags.hpp add9e0a1ec8c73e6556f9da2019e409edda305b3 
>   src/tests/script.cpp 783b369db44ab73132cc0b7d22a3c1f4d1928fda 
>   src/tests/slave_state_tests.cpp 7d09507d37f41c77f32ec04c7ecfc9657f196a30 
>   src/tests/utils.cpp deef0bba1374cf8a8591813877a6170974d6de94 
>   src/zookeeper/group.cpp 876b2ce4e94bfe45902ef86c3143b68ff30029f0 
>   third_party/libprocess/include/process/future.hpp 49767c57c45f2a4d5c293b1ce0dcef3e40ab2e9f 
>   third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6 
>   third_party/libprocess/include/stout/numify.hpp 9f49e80c4f4d366264a699b12f84fc114f2dfe6a 
> 
> Diff: https://reviews.apache.org/r/7772/diff/
> 
> 
> Testing
> -------
> 
> CentOS: make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: Created a CHECK_SOME macro.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7772/
-----------------------------------------------------------

(Updated Dec. 6, 2012, 10 p.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

Updated to be more succinct based on feedback from benh.


Description
-------

This adds consistency with what we've done with ASSERT_SOME, EXPECT_SOME.


Diffs (updated)
-----

  src/Makefile.am b2d1edf140797c7150cb4644d323296965c4f000 
  src/java/jni/org_apache_mesos_Log.cpp e9ffccf8912977ff89cc5cb1ea1369ea5ce25647 
  src/log/coordinator.cpp 31d48e901825b44de0b5e66394cd9020108b45b6 
  src/log/log.hpp db026dc1edcf0bfe059dcff4036a1278d9933c26 
  src/log/log.cpp fcc343ef0e7cab3152d49dd837ce4da0347c8cd8 
  src/log/main.cpp 69d9443702e2ddf3b868957fe8f2712346a677de 
  src/log/replica.cpp b39623e8a9cb44cf4889b4d3351e952227810e54 
  src/logging/check_some.hpp PRE-CREATION 
  src/logging/logging.hpp 8e8a4f6f92644c4db2d0d59925a930b6f0f72961 
  src/master/main.cpp 9e00e738df26af07f5edfbc79a8a2812c57db287 
  src/slave/main.cpp 70bb71e427aaa5e1ebb1ae132c9aad3ebe9be34c 
  src/slave/paths.hpp 98e7fd402919c50a26f69a2f1a1904cb877c5f43 
  src/slave/process_based_isolation_module.cpp ede080069d814f410662f759806d1d5b260e8569 
  src/slave/slave.cpp 28fd4c336d8ac658cf92811d20066a6cfdf5a95e 
  src/slave/state.cpp 65d91b889b84152ed12c4f2dd623b537242258df 
  src/tests/flags.hpp add9e0a1ec8c73e6556f9da2019e409edda305b3 
  src/tests/script.cpp 783b369db44ab73132cc0b7d22a3c1f4d1928fda 
  src/tests/slave_state_tests.cpp 7d09507d37f41c77f32ec04c7ecfc9657f196a30 
  src/tests/utils.cpp deef0bba1374cf8a8591813877a6170974d6de94 
  src/zookeeper/group.cpp 876b2ce4e94bfe45902ef86c3143b68ff30029f0 
  third_party/libprocess/include/process/future.hpp 49767c57c45f2a4d5c293b1ce0dcef3e40ab2e9f 
  third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6 
  third_party/libprocess/include/stout/numify.hpp 9f49e80c4f4d366264a699b12f84fc114f2dfe6a 

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


Testing
-------

CentOS: make check


Thanks,

Ben Mahler


Re: Review Request: Created a CHECK_SOME macro.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7772/
-----------------------------------------------------------

(Updated Dec. 5, 2012, 11:15 p.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

Updated on benh review.


Description
-------

This adds consistency with what we've done with ASSERT_SOME, EXPECT_SOME.


Diffs (updated)
-----

  src/Makefile.am b2d1edf140797c7150cb4644d323296965c4f000 
  src/java/jni/org_apache_mesos_Log.cpp e9ffccf8912977ff89cc5cb1ea1369ea5ce25647 
  src/log/coordinator.cpp 31d48e901825b44de0b5e66394cd9020108b45b6 
  src/log/log.hpp db026dc1edcf0bfe059dcff4036a1278d9933c26 
  src/log/log.cpp fcc343ef0e7cab3152d49dd837ce4da0347c8cd8 
  src/log/main.cpp 69d9443702e2ddf3b868957fe8f2712346a677de 
  src/log/replica.cpp b39623e8a9cb44cf4889b4d3351e952227810e54 
  src/logging/check_some.hpp PRE-CREATION 
  src/logging/logging.hpp 8e8a4f6f92644c4db2d0d59925a930b6f0f72961 
  src/master/main.cpp 9e00e738df26af07f5edfbc79a8a2812c57db287 
  src/slave/main.cpp 70bb71e427aaa5e1ebb1ae132c9aad3ebe9be34c 
  src/slave/paths.hpp 98e7fd402919c50a26f69a2f1a1904cb877c5f43 
  src/slave/process_based_isolation_module.cpp ede080069d814f410662f759806d1d5b260e8569 
  src/slave/slave.cpp 28fd4c336d8ac658cf92811d20066a6cfdf5a95e 
  src/slave/state.cpp 65d91b889b84152ed12c4f2dd623b537242258df 
  src/tests/flags.hpp add9e0a1ec8c73e6556f9da2019e409edda305b3 
  src/tests/script.cpp 783b369db44ab73132cc0b7d22a3c1f4d1928fda 
  src/tests/slave_state_tests.cpp 7d09507d37f41c77f32ec04c7ecfc9657f196a30 
  src/tests/utils.cpp deef0bba1374cf8a8591813877a6170974d6de94 
  src/zookeeper/group.cpp 876b2ce4e94bfe45902ef86c3143b68ff30029f0 
  third_party/libprocess/include/process/future.hpp 49767c57c45f2a4d5c293b1ce0dcef3e40ab2e9f 
  third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6 
  third_party/libprocess/include/stout/numify.hpp 9f49e80c4f4d366264a699b12f84fc114f2dfe6a 

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


Testing
-------

CentOS: make check


Thanks,

Ben Mahler


Re: Review Request: Created a CHECK_SOME macro.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7772/
-----------------------------------------------------------

(Updated Nov. 15, 2012, 7:39 p.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

Benh comments + did a little bit of error string fixing (from Error -> Failed).


Description
-------

This adds consistency with what we've done with ASSERT_SOME, EXPECT_SOME.


Diffs (updated)
-----

  src/Makefile.am b2d1edf140797c7150cb4644d323296965c4f000 
  src/java/jni/org_apache_mesos_Log.cpp e9ffccf8912977ff89cc5cb1ea1369ea5ce25647 
  src/log/coordinator.cpp 31d48e901825b44de0b5e66394cd9020108b45b6 
  src/log/log.hpp db026dc1edcf0bfe059dcff4036a1278d9933c26 
  src/log/log.cpp fcc343ef0e7cab3152d49dd837ce4da0347c8cd8 
  src/log/main.cpp 69d9443702e2ddf3b868957fe8f2712346a677de 
  src/log/replica.cpp b39623e8a9cb44cf4889b4d3351e952227810e54 
  src/logging/check_some.hpp PRE-CREATION 
  src/logging/logging.hpp 8e8a4f6f92644c4db2d0d59925a930b6f0f72961 
  src/master/main.cpp 9e00e738df26af07f5edfbc79a8a2812c57db287 
  src/slave/main.cpp 70bb71e427aaa5e1ebb1ae132c9aad3ebe9be34c 
  src/slave/paths.hpp 98e7fd402919c50a26f69a2f1a1904cb877c5f43 
  src/slave/process_based_isolation_module.cpp 16fd584e78db2c517d828f2576ab8a38c5ce57ad 
  src/slave/slave.cpp 7deb4574943aae4cfc5da5d6b3f600042686975f 
  src/slave/state.cpp 65d91b889b84152ed12c4f2dd623b537242258df 
  src/tests/flags.hpp add9e0a1ec8c73e6556f9da2019e409edda305b3 
  src/tests/script.cpp 783b369db44ab73132cc0b7d22a3c1f4d1928fda 
  src/tests/slave_state_tests.cpp 7d09507d37f41c77f32ec04c7ecfc9657f196a30 
  src/tests/utils.cpp deef0bba1374cf8a8591813877a6170974d6de94 
  src/zookeeper/group.cpp 876b2ce4e94bfe45902ef86c3143b68ff30029f0 
  third_party/libprocess/include/process/future.hpp 49767c57c45f2a4d5c293b1ce0dcef3e40ab2e9f 
  third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6 
  third_party/libprocess/include/stout/numify.hpp 9f49e80c4f4d366264a699b12f84fc114f2dfe6a 

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


Testing
-------

CentOS: make check


Thanks,

Ben Mahler


Re: Review Request: Created a CHECK_SOME macro.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7772/
-----------------------------------------------------------

(Updated Nov. 6, 2012, 11:55 p.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

Here we go.. CHECK_SOME now appends the error reason after the message.

Manually tested all cases (Try some/error, Option some/none, Result some/error/none).


Description
-------

This adds consistency with what we've done with ASSERT_SOME, EXPECT_SOME.


Diffs (updated)
-----

  src/java/jni/org_apache_mesos_Log.cpp e9ffccf8912977ff89cc5cb1ea1369ea5ce25647 
  src/log/coordinator.cpp 31d48e901825b44de0b5e66394cd9020108b45b6 
  src/log/log.hpp db026dc1edcf0bfe059dcff4036a1278d9933c26 
  src/log/log.cpp fcc343ef0e7cab3152d49dd837ce4da0347c8cd8 
  src/log/main.cpp 69d9443702e2ddf3b868957fe8f2712346a677de 
  src/log/replica.cpp b39623e8a9cb44cf4889b4d3351e952227810e54 
  src/logging/logging.hpp 8e8a4f6f92644c4db2d0d59925a930b6f0f72961 
  src/master/main.cpp 74c955f262179d52408d6b0eaada4d7801bcc193 
  src/slave/main.cpp 70bb71e427aaa5e1ebb1ae132c9aad3ebe9be34c 
  src/slave/paths.hpp 98e7fd402919c50a26f69a2f1a1904cb877c5f43 
  src/slave/process_based_isolation_module.cpp b0b6a81c93acc68d1f4acbdda5ab2f9f96b5fb5a 
  src/slave/slave.cpp 2bd2dbce538a6108dd9fe607829cfbdab33e0778 
  src/slave/state.cpp 65d91b889b84152ed12c4f2dd623b537242258df 
  src/tests/flags.hpp add9e0a1ec8c73e6556f9da2019e409edda305b3 
  src/tests/script.cpp 783b369db44ab73132cc0b7d22a3c1f4d1928fda 
  src/tests/slave_state_tests.cpp 7d09507d37f41c77f32ec04c7ecfc9657f196a30 
  src/tests/utils.cpp deef0bba1374cf8a8591813877a6170974d6de94 
  src/zookeeper/group.cpp 876b2ce4e94bfe45902ef86c3143b68ff30029f0 
  third_party/libprocess/include/process/future.hpp 49767c57c45f2a4d5c293b1ce0dcef3e40ab2e9f 
  third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6 
  third_party/libprocess/include/stout/numify.hpp 9f49e80c4f4d366264a699b12f84fc114f2dfe6a 

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


Testing
-------

CentOS: make check


Thanks,

Ben Mahler