You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Hindman <be...@berkeley.edu> on 2017/07/27 01:55:21 UTC
Review Request 61149: Added Future::condition.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61149/
-----------------------------------------------------------
Review request for mesos and Benjamin Mahler.
Repository: mesos
Description
-------
Added Future::condition.
Diffs
-----
3rdparty/libprocess/include/process/future.hpp cce950509f58022e79bb51a6e72ea1a005b9cb50
Diff: https://reviews.apache.org/r/61149/diff/1/
Testing
-------
make check
Thanks,
Benjamin Hindman
Re: Review Request 61149: Added stringification support for Future.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61149/#review191464
-----------------------------------------------------------
3rdparty/libprocess/include/process/future.hpp
Lines 1782-1804 (patched)
<https://reviews.apache.org/r/61149/#comment269287>
Something like this?
```
const std::string suffix = future.data->discard ? " (with discard)" : "";
switch (future.data->state) {
case Future<T>::PENDING:
if (future.data->abandoned) {
return stream << "Abandoned" << suffix;
}
return stream << "Pending" << suffix;
case Future<T>::READY:
// TODO(benh): Stringify `Future<T>::get()` if it can be
// stringified (will need to be SFINAE'ed appropriately).
return stream << "Ready" << suffix;
case Future<T>::FAILED:
return stream << "Failed" << suffix << ": " << future.failure();
case Future<T>::DISCARDED:
return stream << "Discarded" << suffix;
}
```
- Benjamin Mahler
On July 27, 2017, 1:55 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61149/
> -----------------------------------------------------------
>
> (Updated July 27, 2017, 1:55 a.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This is especially useful when creating error messages and the future
> has failed or been discarded.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/future.hpp a11a588941b02d776da2f563dd246a92dfbbc360
> 3rdparty/libprocess/src/tests/future_tests.cpp 76a32bd69499e52ea1623ab982d65e1c7a0cbd32
>
>
> Diff: https://reviews.apache.org/r/61149/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 61149: Added Future::condition.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61149/#review183221
-----------------------------------------------------------
Looks like the summary became incorrect after the last diff update.
- Alexander Rukletsov
On July 27, 2017, 1:55 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61149/
> -----------------------------------------------------------
>
> (Updated July 27, 2017, 1:55 a.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added Future::condition.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/future.hpp cce950509f58022e79bb51a6e72ea1a005b9cb50
> 3rdparty/libprocess/src/tests/future_tests.cpp 0c8725b9a5e64aaac6e3979e450a11e84f9bd45e
>
>
> Diff: https://reviews.apache.org/r/61149/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 61149: Added Future::condition.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61149/#review181641
-----------------------------------------------------------
3rdparty/libprocess/include/process/future.hpp
Lines 155-156 (patched)
<https://reviews.apache.org/r/61149/#comment257268>
I'm not sure callers will grasp this, to me this is stringification of the future, and in the READY case it would print `'Ready with X'`. Is there a way to acheive this when T isn't printable (e.g. `'Ready with <unprintable value>'`
- Benjamin Mahler
On July 27, 2017, 1:55 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61149/
> -----------------------------------------------------------
>
> (Updated July 27, 2017, 1:55 a.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added Future::condition.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/future.hpp cce950509f58022e79bb51a6e72ea1a005b9cb50
>
>
> Diff: https://reviews.apache.org/r/61149/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 61149: Added stringification support for Future.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61149/#review191463
-----------------------------------------------------------
Ship it!
3rdparty/libprocess/include/process/future.hpp
Lines 1785-1786 (patched)
<https://reviews.apache.org/r/61149/#comment269285>
Any reason not to?
```
return stream << "Abandoned" << suffix;
```
3rdparty/libprocess/include/process/future.hpp
Lines 1790-1792 (patched)
<https://reviews.apache.org/r/61149/#comment269286>
Likewise these cases could be more succinct with returns and no breaks?
```
case Future<T>::PENDING:
return stream << "Pending" << suffix;
case ...
```
- Benjamin Mahler
On July 27, 2017, 1:55 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61149/
> -----------------------------------------------------------
>
> (Updated July 27, 2017, 1:55 a.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This is especially useful when creating error messages and the future
> has failed or been discarded.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/future.hpp a11a588941b02d776da2f563dd246a92dfbbc360
> 3rdparty/libprocess/src/tests/future_tests.cpp 76a32bd69499e52ea1623ab982d65e1c7a0cbd32
>
>
> Diff: https://reviews.apache.org/r/61149/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>