You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Benjamin Hindman <be...@berkeley.edu> on 2013/02/05 06:57:43 UTC
Review Request: Refactored all Try::error and Result::error to use
Error or ErrnoError (also changed a few CHECKs into CHECK_SOMEs).
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9306/
-----------------------------------------------------------
Review request for mesos, Vinod Kone and Ben Mahler.
Description
-------
See summary.
Diffs
-----
src/common/values.cpp 8aebc0b57de110e14c55c5b6d543f17165cfd397
src/detector/detector.cpp ca921151a1fc1a57d265b3439d07a8c45e920f21
src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813
src/flags/parse.hpp df7e3480c1f5422d185c86cc5f34e7b87b024a2f
src/linux/cgroups.cpp 03b31e7309b9dd65f00d3b0da2abb81ddaaeea43
src/linux/fs.cpp ea74c3b935dc716fd53827c8a7eb7010961e7aa6
src/linux/proc.cpp 8a0fc48dc9769df35d682ece477246b2df2fc0d4
src/log/coordinator.cpp d9f67bd8b07e6f8b3e022c21f1764d298e314594
src/log/log.hpp 151eebd5d665993dfaac399553a9a315c093762b
src/log/replica.cpp 392cb15991657d8e79637e1c4bc21ec813e3117d
src/logging/logging.cpp e9d778f840f2d6713dbb99d0a8844033be9f0e5f
src/master/frameworks_manager.cpp 189a7e3b4811643baf285ad29f939453257c705f
src/sched/sched.cpp f1eeab6f12ee300d77013c6a4ba62ccd7fdb0d1d
src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36
src/state/leveldb.cpp 6137b5d089dc9ea4b60fd56153b72e14691df3d8
src/state/serializer.hpp 1915cefb102526fbad330b090b8c788bea65ed84
src/state/zookeeper.cpp 4c684797791bb2158d31573d2d47b0da70283bc6
src/tests/master_tests.cpp 948ab5dff34eeba1f3ce593a864ddf282c8b19ed
src/zookeeper/group.cpp e74538e3dfa6951178f3e4b49dfd94374a0aa28c
src/zookeeper/url.hpp c62539b30503a94ed82e11fe84d988c122d36bb6
third_party/libprocess/include/process/http.hpp 40248dab3df59b322ad9c5a4858961ef471779fd
third_party/libprocess/include/stout/duration.hpp caf7ce42b34dfb74206d1f10aafabaaec47e7dfc
third_party/libprocess/include/stout/error.hpp PRE-CREATION
third_party/libprocess/include/stout/format.hpp fc349903d31d6e31a8147e3a6be87b37698f28f0
third_party/libprocess/include/stout/fs.hpp 9e62a1b91bc9fac092818ffb3c8bcec46b0bd26d
third_party/libprocess/include/stout/gzip.hpp 6ff1288683a0d3b05098c772945cc2a8b73f8b01
third_party/libprocess/include/stout/net.hpp 1909046017da005a5e7c64d525bb3c741d5ad436
third_party/libprocess/include/stout/numify.hpp 2a59c9269902541d82ba86a2d159984f5bbc6b73
third_party/libprocess/include/stout/os.hpp 712fb209061384b8c045875ef8898a6efd778514
third_party/libprocess/include/stout/protobuf.hpp 25d4634bbc51d14ea78c1a922353f0ce52e458df
Diff: https://reviews.apache.org/r/9306/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman
Re: Review Request: Refactored all Try::error and Result::error to use
Error or ErrnoError (also changed a few CHECKs into CHECK_SOMEs).
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On Feb. 5, 2013, 10:27 p.m., Ben Mahler wrote:
> > This is SO MUCH CLEANER!
> >
> > Two notes:
> > (1) Seems like Error could also take a second argument:
> > Error(string m, Try<X> try) : message(m + ": " + try.error())
> >
> > But that gets a bit implicit:
> > Try<string> read = os::read("/foo");
> > return Error("Failed to read '/foo'", read);
> > vs.
> > return Error("Failed to read '/foo': " + read.error());
> >
> > (2) Nearly all of my comments here are for additional cleanup in the existing code. It would be awesome if you got all these, but feel free to drop if it's too much.
Regardless of the implicitness here, what would be the semantics if 'read' actually isn't an error? (Or is that what you meant by 'implicit'?)
> On Feb. 5, 2013, 10:27 p.m., Ben Mahler wrote:
> > src/common/values.cpp, line 56
> > <https://reviews.apache.org/r/9306/diff/1/?file=255493#file255493line56>
> >
> > Maybe in this review we should clean up these messages? They seem redundant and inconsistent as they are now.
> >
> > I think it's fair to expect the caller to print this out as:
> >
> > LOG(ERROR) << "Failed to parse '" << text << "': " << parse.error();
> >
> > In which case the current messages won't read so nicely.
Done and done.
> On Feb. 5, 2013, 10:27 p.m., Ben Mahler wrote:
> > src/files/files.cpp, line 417
> > <https://reviews.apache.org/r/9306/diff/1/?file=255495#file255495line417>
> >
> > I should have made this:
> >
> > s/result/realpath
> > "Failed to get realpath: " + realpath.error()
I did "Failed to determine canonical path ...".
> On Feb. 5, 2013, 10:27 p.m., Ben Mahler wrote:
> > src/linux/cgroups.cpp, line 104
> > <https://reviews.apache.org/r/9306/diff/1/?file=255497#file255497line104>
> >
> > too bad ifstream doesn't provide something akin to strerror
Added a TODO to switch to os::read for better error information.
> On Feb. 5, 2013, 10:27 p.m., Ben Mahler wrote:
> > src/linux/cgroups.cpp, line 323
> > <https://reviews.apache.org/r/9306/diff/1/?file=255497#file255497line323>
> >
> > FWICT, It turns out it's not part of the spec!
> >
> > If we really want the strerror message, then we can't use ifstream. I think we do want strerror, given how many places here we just print an error message without knowing why it fails.
> >
> > Maybe a TODO since this happens in many places in the cgroups code.
As it turns out there is already a comment here explaining that we can't use os::read because it does lseek. As we've talked about before, we need an implementation of os::read(path) which does the "expected" semantics of just reading data until EOF. I've added a TODO here nonetheless.
> On Feb. 5, 2013, 10:27 p.m., Ben Mahler wrote:
> > src/linux/cgroups.cpp, line 677
> > <https://reviews.apache.org/r/9306/diff/1/?file=255497#file255497line677>
> >
> > this Option<string> reads backwards from all other error checking, TODO to fix?
Ha, we talked about this before ... check out the old thread. The TL;DR; is that this is supposed to really act like a "macro" that many of these functions needs to perform. The verify routine returns _some_ fully constructed error string if there is an error that can be directly returned via Error(error.get()). I didn't like the idea of returning a Try<Nothing> and then pulling out the error and returning it via Error(verification.error()) because we try and always prepend something to the error string (e.g., Error("Failed to verify: " + verification.error())) and in these cases we wouldn't/shouldn't need to. Given your reaction here, it sounds like this should get changed, but I'm not going to take that on in this review.
> On Feb. 5, 2013, 10:27 p.m., Ben Mahler wrote:
> > src/linux/cgroups.cpp, line 769
> > <https://reviews.apache.org/r/9306/diff/1/?file=255497#file255497line769>
> >
> > s/a/the/ ?
Changed to "Failed to stop traversing file system" (in above and below errors too).
> On Feb. 5, 2013, 10:27 p.m., Ben Mahler wrote:
> > src/slave/cgroups_isolation_module.cpp, line 281
> > <https://reviews.apache.org/r/9306/diff/1/?file=255506#file255506line281>
> >
> > single quote instead and single quote the hierarchy?
> On Feb. 5, 2013, 10:27 p.m., Ben Mahler wrote:
> > third_party/libprocess/include/stout/net.hpp, line 30
> > <https://reviews.apache.org/r/9306/diff/1/?file=255519#file255519line30>
> >
> > Can we add "because libcurl is not available"?
I changed the entire comment to "libcurl is not available", so a concatenated message will probably read: "Failed to download 'resource': libcurl is not available".
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9306/#review16134
-----------------------------------------------------------
On Feb. 8, 2013, 5:49 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9306/
> -----------------------------------------------------------
>
> (Updated Feb. 8, 2013, 5:49 a.m.)
>
>
> Review request for mesos, Vinod Kone and Ben Mahler.
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/common/values.cpp 8aebc0b57de110e14c55c5b6d543f17165cfd397
> src/detector/detector.cpp ca921151a1fc1a57d265b3439d07a8c45e920f21
> src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813
> src/flags/parse.hpp df7e3480c1f5422d185c86cc5f34e7b87b024a2f
> src/linux/cgroups.cpp 03b31e7309b9dd65f00d3b0da2abb81ddaaeea43
> src/linux/fs.cpp ea74c3b935dc716fd53827c8a7eb7010961e7aa6
> src/linux/proc.cpp 8a0fc48dc9769df35d682ece477246b2df2fc0d4
> src/log/coordinator.cpp d9f67bd8b07e6f8b3e022c21f1764d298e314594
> src/log/log.hpp 151eebd5d665993dfaac399553a9a315c093762b
> src/log/replica.cpp 392cb15991657d8e79637e1c4bc21ec813e3117d
> src/logging/logging.cpp e9d778f840f2d6713dbb99d0a8844033be9f0e5f
> src/master/frameworks_manager.cpp 189a7e3b4811643baf285ad29f939453257c705f
> src/sched/sched.cpp f1eeab6f12ee300d77013c6a4ba62ccd7fdb0d1d
> src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36
> src/state/leveldb.cpp 6137b5d089dc9ea4b60fd56153b72e14691df3d8
> src/state/serializer.hpp 1915cefb102526fbad330b090b8c788bea65ed84
> src/state/zookeeper.cpp 4c684797791bb2158d31573d2d47b0da70283bc6
> src/tests/master_tests.cpp 948ab5dff34eeba1f3ce593a864ddf282c8b19ed
> src/tests/stout_tests.cpp fffef3e42774e9b090a098a219a92e642d303d27
> src/zookeeper/group.cpp e74538e3dfa6951178f3e4b49dfd94374a0aa28c
> src/zookeeper/url.hpp c62539b30503a94ed82e11fe84d988c122d36bb6
> third_party/libprocess/include/process/http.hpp 40248dab3df59b322ad9c5a4858961ef471779fd
> third_party/libprocess/include/stout/duration.hpp caf7ce42b34dfb74206d1f10aafabaaec47e7dfc
> third_party/libprocess/include/stout/error.hpp PRE-CREATION
> third_party/libprocess/include/stout/format.hpp fc349903d31d6e31a8147e3a6be87b37698f28f0
> third_party/libprocess/include/stout/fs.hpp 9e62a1b91bc9fac092818ffb3c8bcec46b0bd26d
> third_party/libprocess/include/stout/gzip.hpp 6ff1288683a0d3b05098c772945cc2a8b73f8b01
> third_party/libprocess/include/stout/net.hpp 1909046017da005a5e7c64d525bb3c741d5ad436
> third_party/libprocess/include/stout/numify.hpp 2a59c9269902541d82ba86a2d159984f5bbc6b73
> third_party/libprocess/include/stout/os.hpp 712fb209061384b8c045875ef8898a6efd778514
> third_party/libprocess/include/stout/protobuf.hpp 25d4634bbc51d14ea78c1a922353f0ce52e458df
>
> Diff: https://reviews.apache.org/r/9306/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request: Refactored all Try::error and Result::error to use
Error or ErrnoError (also changed a few CHECKs into CHECK_SOMEs).
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9306/#review16134
-----------------------------------------------------------
This is SO MUCH CLEANER!
Two notes:
(1) Seems like Error could also take a second argument:
Error(string m, Try<X> try) : message(m + ": " + try.error())
But that gets a bit implicit:
Try<string> read = os::read("/foo");
return Error("Failed to read '/foo'", read);
vs.
return Error("Failed to read '/foo': " + read.error());
(2) Nearly all of my comments here are for additional cleanup in the existing code. It would be awesome if you got all these, but feel free to drop if it's too much.
src/common/values.cpp
<https://reviews.apache.org/r/9306/#comment34488>
Maybe in this review we should clean up these messages? They seem redundant and inconsistent as they are now.
I think it's fair to expect the caller to print this out as:
LOG(ERROR) << "Failed to parse '" << text << "': " << parse.error();
In which case the current messages won't read so nicely.
src/files/files.cpp
<https://reviews.apache.org/r/9306/#comment34490>
I should have made this:
s/result/realpath
"Failed to get realpath: " + realpath.error()
src/linux/cgroups.cpp
<https://reviews.apache.org/r/9306/#comment34491>
too bad ifstream doesn't provide something akin to strerror
src/linux/cgroups.cpp
<https://reviews.apache.org/r/9306/#comment34492>
s/directory at "/directory '"/
s/": "/"': "/
src/linux/cgroups.cpp
<https://reviews.apache.org/r/9306/#comment34494>
s/at "/'"/
s/": "/"': "/
src/linux/cgroups.cpp
<https://reviews.apache.org/r/9306/#comment34495>
FWICT, It turns out it's not part of the spec!
If we really want the strerror message, then we can't use ifstream. I think we do want strerror, given how many places here we just print an error message without knowing why it fails.
Maybe a TODO since this happens in many places in the cgroups code.
src/linux/cgroups.cpp
<https://reviews.apache.org/r/9306/#comment34497>
single quotes around hierarchy
src/linux/cgroups.cpp
<https://reviews.apache.org/r/9306/#comment34496>
single quotes around dir
src/linux/cgroups.cpp
<https://reviews.apache.org/r/9306/#comment34498>
single quotes around hierarchy
src/linux/cgroups.cpp
<https://reviews.apache.org/r/9306/#comment34499>
single quotes around hierarchy
src/linux/cgroups.cpp
<https://reviews.apache.org/r/9306/#comment34500>
ditto
src/linux/cgroups.cpp
<https://reviews.apache.org/r/9306/#comment34501>
ditto
src/linux/cgroups.cpp
<https://reviews.apache.org/r/9306/#comment34503>
this Option<string> reads backwards from all other error checking, TODO to fix?
src/linux/cgroups.cpp
<https://reviews.apache.org/r/9306/#comment34504>
s/a/the/ ?
src/linux/cgroups.cpp
<https://reviews.apache.org/r/9306/#comment34505>
Should we include value.get() here at least?
src/linux/fs.cpp
<https://reviews.apache.org/r/9306/#comment34506>
single quote path
src/linux/fs.cpp
<https://reviews.apache.org/r/9306/#comment34507>
single quote these?
src/linux/fs.cpp
<https://reviews.apache.org/r/9306/#comment34509>
single quote target?
src/linux/proc.cpp
<https://reviews.apache.org/r/9306/#comment34512>
single quote?
src/linux/proc.cpp
<https://reviews.apache.org/r/9306/#comment34514>
single quote?
src/master/frameworks_manager.cpp
<https://reviews.apache.org/r/9306/#comment34515>
"Framework does not exist."?
src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/9306/#comment34516>
Thanks for catching these CHECK_SOMEs!
single quote?
src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/9306/#comment34517>
single quote?
src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/9306/#comment34518>
single quote instead and single quote the hierarchy?
src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/9306/#comment34519>
single quote instead?
src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/9306/#comment34520>
single quote instead?
src/slave/cgroups_isolation_module.cpp
<https://reviews.apache.org/r/9306/#comment34521>
single quote instead?
src/state/zookeeper.cpp
<https://reviews.apache.org/r/9306/#comment34523>
Is compression easy now with gzip available? Perhaps update this TODO to reflect that?
src/zookeeper/group.cpp
<https://reviews.apache.org/r/9306/#comment34524>
s/base/basename/ ?
third_party/libprocess/include/stout/gzip.hpp
<https://reviews.apache.org/r/9306/#comment34525>
"Failed to initialize zlib: " + ...
third_party/libprocess/include/stout/gzip.hpp
<https://reviews.apache.org/r/9306/#comment34526>
"Failed to clean up zlib: " + ...
third_party/libprocess/include/stout/gzip.hpp
<https://reviews.apache.org/r/9306/#comment34527>
"Failed to initialize zlib: " + ...
third_party/libprocess/include/stout/gzip.hpp
<https://reviews.apache.org/r/9306/#comment34528>
"Failed to clean up zlib: " + ...
third_party/libprocess/include/stout/net.hpp
<https://reviews.apache.org/r/9306/#comment34529>
Can we add "because libcurl is not available"?
third_party/libprocess/include/stout/numify.hpp
<https://reviews.apache.org/r/9306/#comment34530>
thanks!
third_party/libprocess/include/stout/os.hpp
<https://reviews.apache.org/r/9306/#comment34531>
single quotes
third_party/libprocess/include/stout/os.hpp
<https://reviews.apache.org/r/9306/#comment34534>
single quotes?
third_party/libprocess/include/stout/os.hpp
<https://reviews.apache.org/r/9306/#comment34536>
This is more of 'directory' not being a directory. It may still exist (as a file, link, etc).
third_party/libprocess/include/stout/os.hpp
<https://reviews.apache.org/r/9306/#comment34537>
Add "on non-linux systems" ?
third_party/libprocess/include/stout/protobuf.hpp
<https://reviews.apache.org/r/9306/#comment34540>
single quotes
- Ben Mahler
On Feb. 5, 2013, 5:57 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9306/
> -----------------------------------------------------------
>
> (Updated Feb. 5, 2013, 5:57 a.m.)
>
>
> Review request for mesos, Vinod Kone and Ben Mahler.
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/common/values.cpp 8aebc0b57de110e14c55c5b6d543f17165cfd397
> src/detector/detector.cpp ca921151a1fc1a57d265b3439d07a8c45e920f21
> src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813
> src/flags/parse.hpp df7e3480c1f5422d185c86cc5f34e7b87b024a2f
> src/linux/cgroups.cpp 03b31e7309b9dd65f00d3b0da2abb81ddaaeea43
> src/linux/fs.cpp ea74c3b935dc716fd53827c8a7eb7010961e7aa6
> src/linux/proc.cpp 8a0fc48dc9769df35d682ece477246b2df2fc0d4
> src/log/coordinator.cpp d9f67bd8b07e6f8b3e022c21f1764d298e314594
> src/log/log.hpp 151eebd5d665993dfaac399553a9a315c093762b
> src/log/replica.cpp 392cb15991657d8e79637e1c4bc21ec813e3117d
> src/logging/logging.cpp e9d778f840f2d6713dbb99d0a8844033be9f0e5f
> src/master/frameworks_manager.cpp 189a7e3b4811643baf285ad29f939453257c705f
> src/sched/sched.cpp f1eeab6f12ee300d77013c6a4ba62ccd7fdb0d1d
> src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36
> src/state/leveldb.cpp 6137b5d089dc9ea4b60fd56153b72e14691df3d8
> src/state/serializer.hpp 1915cefb102526fbad330b090b8c788bea65ed84
> src/state/zookeeper.cpp 4c684797791bb2158d31573d2d47b0da70283bc6
> src/tests/master_tests.cpp 948ab5dff34eeba1f3ce593a864ddf282c8b19ed
> src/zookeeper/group.cpp e74538e3dfa6951178f3e4b49dfd94374a0aa28c
> src/zookeeper/url.hpp c62539b30503a94ed82e11fe84d988c122d36bb6
> third_party/libprocess/include/process/http.hpp 40248dab3df59b322ad9c5a4858961ef471779fd
> third_party/libprocess/include/stout/duration.hpp caf7ce42b34dfb74206d1f10aafabaaec47e7dfc
> third_party/libprocess/include/stout/error.hpp PRE-CREATION
> third_party/libprocess/include/stout/format.hpp fc349903d31d6e31a8147e3a6be87b37698f28f0
> third_party/libprocess/include/stout/fs.hpp 9e62a1b91bc9fac092818ffb3c8bcec46b0bd26d
> third_party/libprocess/include/stout/gzip.hpp 6ff1288683a0d3b05098c772945cc2a8b73f8b01
> third_party/libprocess/include/stout/net.hpp 1909046017da005a5e7c64d525bb3c741d5ad436
> third_party/libprocess/include/stout/numify.hpp 2a59c9269902541d82ba86a2d159984f5bbc6b73
> third_party/libprocess/include/stout/os.hpp 712fb209061384b8c045875ef8898a6efd778514
> third_party/libprocess/include/stout/protobuf.hpp 25d4634bbc51d14ea78c1a922353f0ce52e458df
>
> Diff: https://reviews.apache.org/r/9306/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request: Refactored all Try::error and Result::error to use
Error or ErrnoError (also changed a few CHECKs into CHECK_SOMEs).
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On Feb. 8, 2013, 9:31 p.m., Ben Mahler wrote:
> > src/linux/cgroups.cpp, line 377
> > <https://reviews.apache.org/r/9306/diff/1-2/?file=255497#file255497line377>
> >
> > Can't the implicit constructor work in here?
I'm going to get the 'Some' calls in a future review.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9306/#review16371
-----------------------------------------------------------
On Feb. 8, 2013, 5:49 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9306/
> -----------------------------------------------------------
>
> (Updated Feb. 8, 2013, 5:49 a.m.)
>
>
> Review request for mesos, Vinod Kone and Ben Mahler.
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/common/values.cpp 8aebc0b57de110e14c55c5b6d543f17165cfd397
> src/detector/detector.cpp ca921151a1fc1a57d265b3439d07a8c45e920f21
> src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813
> src/flags/parse.hpp df7e3480c1f5422d185c86cc5f34e7b87b024a2f
> src/linux/cgroups.cpp 03b31e7309b9dd65f00d3b0da2abb81ddaaeea43
> src/linux/fs.cpp ea74c3b935dc716fd53827c8a7eb7010961e7aa6
> src/linux/proc.cpp 8a0fc48dc9769df35d682ece477246b2df2fc0d4
> src/log/coordinator.cpp d9f67bd8b07e6f8b3e022c21f1764d298e314594
> src/log/log.hpp 151eebd5d665993dfaac399553a9a315c093762b
> src/log/replica.cpp 392cb15991657d8e79637e1c4bc21ec813e3117d
> src/logging/logging.cpp e9d778f840f2d6713dbb99d0a8844033be9f0e5f
> src/master/frameworks_manager.cpp 189a7e3b4811643baf285ad29f939453257c705f
> src/sched/sched.cpp f1eeab6f12ee300d77013c6a4ba62ccd7fdb0d1d
> src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36
> src/state/leveldb.cpp 6137b5d089dc9ea4b60fd56153b72e14691df3d8
> src/state/serializer.hpp 1915cefb102526fbad330b090b8c788bea65ed84
> src/state/zookeeper.cpp 4c684797791bb2158d31573d2d47b0da70283bc6
> src/tests/master_tests.cpp 948ab5dff34eeba1f3ce593a864ddf282c8b19ed
> src/tests/stout_tests.cpp fffef3e42774e9b090a098a219a92e642d303d27
> src/zookeeper/group.cpp e74538e3dfa6951178f3e4b49dfd94374a0aa28c
> src/zookeeper/url.hpp c62539b30503a94ed82e11fe84d988c122d36bb6
> third_party/libprocess/include/process/http.hpp 40248dab3df59b322ad9c5a4858961ef471779fd
> third_party/libprocess/include/stout/duration.hpp caf7ce42b34dfb74206d1f10aafabaaec47e7dfc
> third_party/libprocess/include/stout/error.hpp PRE-CREATION
> third_party/libprocess/include/stout/format.hpp fc349903d31d6e31a8147e3a6be87b37698f28f0
> third_party/libprocess/include/stout/fs.hpp 9e62a1b91bc9fac092818ffb3c8bcec46b0bd26d
> third_party/libprocess/include/stout/gzip.hpp 6ff1288683a0d3b05098c772945cc2a8b73f8b01
> third_party/libprocess/include/stout/net.hpp 1909046017da005a5e7c64d525bb3c741d5ad436
> third_party/libprocess/include/stout/numify.hpp 2a59c9269902541d82ba86a2d159984f5bbc6b73
> third_party/libprocess/include/stout/os.hpp 712fb209061384b8c045875ef8898a6efd778514
> third_party/libprocess/include/stout/protobuf.hpp 25d4634bbc51d14ea78c1a922353f0ce52e458df
>
> Diff: https://reviews.apache.org/r/9306/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request: Refactored all Try::error and Result::error to use
Error or ErrnoError (also changed a few CHECKs into CHECK_SOMEs).
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9306/#review16371
-----------------------------------------------------------
Ship it!
src/linux/cgroups.cpp
<https://reviews.apache.org/r/9306/#comment34838>
Can't the implicit constructor work in here?
src/linux/cgroups.cpp
<https://reviews.apache.org/r/9306/#comment34839>
ditto
src/linux/cgroups.cpp
<https://reviews.apache.org/r/9306/#comment34840>
ditto
src/linux/cgroups.cpp
<https://reviews.apache.org/r/9306/#comment34841>
ditto
third_party/libprocess/include/stout/gzip.hpp
<https://reviews.apache.org/r/9306/#comment34842>
thanks
- Ben Mahler
On Feb. 8, 2013, 5:49 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9306/
> -----------------------------------------------------------
>
> (Updated Feb. 8, 2013, 5:49 a.m.)
>
>
> Review request for mesos, Vinod Kone and Ben Mahler.
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/common/values.cpp 8aebc0b57de110e14c55c5b6d543f17165cfd397
> src/detector/detector.cpp ca921151a1fc1a57d265b3439d07a8c45e920f21
> src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813
> src/flags/parse.hpp df7e3480c1f5422d185c86cc5f34e7b87b024a2f
> src/linux/cgroups.cpp 03b31e7309b9dd65f00d3b0da2abb81ddaaeea43
> src/linux/fs.cpp ea74c3b935dc716fd53827c8a7eb7010961e7aa6
> src/linux/proc.cpp 8a0fc48dc9769df35d682ece477246b2df2fc0d4
> src/log/coordinator.cpp d9f67bd8b07e6f8b3e022c21f1764d298e314594
> src/log/log.hpp 151eebd5d665993dfaac399553a9a315c093762b
> src/log/replica.cpp 392cb15991657d8e79637e1c4bc21ec813e3117d
> src/logging/logging.cpp e9d778f840f2d6713dbb99d0a8844033be9f0e5f
> src/master/frameworks_manager.cpp 189a7e3b4811643baf285ad29f939453257c705f
> src/sched/sched.cpp f1eeab6f12ee300d77013c6a4ba62ccd7fdb0d1d
> src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36
> src/state/leveldb.cpp 6137b5d089dc9ea4b60fd56153b72e14691df3d8
> src/state/serializer.hpp 1915cefb102526fbad330b090b8c788bea65ed84
> src/state/zookeeper.cpp 4c684797791bb2158d31573d2d47b0da70283bc6
> src/tests/master_tests.cpp 948ab5dff34eeba1f3ce593a864ddf282c8b19ed
> src/tests/stout_tests.cpp fffef3e42774e9b090a098a219a92e642d303d27
> src/zookeeper/group.cpp e74538e3dfa6951178f3e4b49dfd94374a0aa28c
> src/zookeeper/url.hpp c62539b30503a94ed82e11fe84d988c122d36bb6
> third_party/libprocess/include/process/http.hpp 40248dab3df59b322ad9c5a4858961ef471779fd
> third_party/libprocess/include/stout/duration.hpp caf7ce42b34dfb74206d1f10aafabaaec47e7dfc
> third_party/libprocess/include/stout/error.hpp PRE-CREATION
> third_party/libprocess/include/stout/format.hpp fc349903d31d6e31a8147e3a6be87b37698f28f0
> third_party/libprocess/include/stout/fs.hpp 9e62a1b91bc9fac092818ffb3c8bcec46b0bd26d
> third_party/libprocess/include/stout/gzip.hpp 6ff1288683a0d3b05098c772945cc2a8b73f8b01
> third_party/libprocess/include/stout/net.hpp 1909046017da005a5e7c64d525bb3c741d5ad436
> third_party/libprocess/include/stout/numify.hpp 2a59c9269902541d82ba86a2d159984f5bbc6b73
> third_party/libprocess/include/stout/os.hpp 712fb209061384b8c045875ef8898a6efd778514
> third_party/libprocess/include/stout/protobuf.hpp 25d4634bbc51d14ea78c1a922353f0ce52e458df
>
> Diff: https://reviews.apache.org/r/9306/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request: Refactored all Try::error and Result::error to use
Error or ErrnoError (also changed a few CHECKs into CHECK_SOMEs).
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9306/
-----------------------------------------------------------
(Updated Feb. 20, 2013, 10:35 p.m.)
Review request for mesos, Vinod Kone and Ben Mahler.
Changes
-------
Addressed comments.
Description
-------
See summary.
Diffs (updated)
-----
src/common/values.cpp 8aebc0b57de110e14c55c5b6d543f17165cfd397
src/detector/detector.cpp ca921151a1fc1a57d265b3439d07a8c45e920f21
src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813
src/flags/parse.hpp df7e3480c1f5422d185c86cc5f34e7b87b024a2f
src/linux/cgroups.cpp 03b31e7309b9dd65f00d3b0da2abb81ddaaeea43
src/linux/fs.cpp ea74c3b935dc716fd53827c8a7eb7010961e7aa6
src/linux/proc.cpp 8a0fc48dc9769df35d682ece477246b2df2fc0d4
src/log/coordinator.cpp d9f67bd8b07e6f8b3e022c21f1764d298e314594
src/log/log.hpp 151eebd5d665993dfaac399553a9a315c093762b
src/log/replica.cpp 392cb15991657d8e79637e1c4bc21ec813e3117d
src/logging/logging.cpp e9d778f840f2d6713dbb99d0a8844033be9f0e5f
src/master/frameworks_manager.cpp 189a7e3b4811643baf285ad29f939453257c705f
src/sched/sched.cpp f1eeab6f12ee300d77013c6a4ba62ccd7fdb0d1d
src/slave/cgroups_isolation_module.cpp 14f549edaf1b37a6bca8f75309864333ae775e7c
src/state/leveldb.cpp 6137b5d089dc9ea4b60fd56153b72e14691df3d8
src/state/serializer.hpp 1915cefb102526fbad330b090b8c788bea65ed84
src/state/zookeeper.cpp 4c684797791bb2158d31573d2d47b0da70283bc6
src/tests/master_tests.cpp 948ab5dff34eeba1f3ce593a864ddf282c8b19ed
src/tests/stout_tests.cpp fffef3e42774e9b090a098a219a92e642d303d27
src/zookeeper/group.cpp e74538e3dfa6951178f3e4b49dfd94374a0aa28c
src/zookeeper/url.hpp c62539b30503a94ed82e11fe84d988c122d36bb6
third_party/libprocess/include/process/http.hpp 40248dab3df59b322ad9c5a4858961ef471779fd
third_party/libprocess/include/stout/duration.hpp caf7ce42b34dfb74206d1f10aafabaaec47e7dfc
third_party/libprocess/include/stout/error.hpp PRE-CREATION
third_party/libprocess/include/stout/format.hpp fc349903d31d6e31a8147e3a6be87b37698f28f0
third_party/libprocess/include/stout/fs.hpp 9e62a1b91bc9fac092818ffb3c8bcec46b0bd26d
third_party/libprocess/include/stout/gzip.hpp 6ff1288683a0d3b05098c772945cc2a8b73f8b01
third_party/libprocess/include/stout/net.hpp 1909046017da005a5e7c64d525bb3c741d5ad436
third_party/libprocess/include/stout/numify.hpp 2a59c9269902541d82ba86a2d159984f5bbc6b73
third_party/libprocess/include/stout/os.hpp 712fb209061384b8c045875ef8898a6efd778514
third_party/libprocess/include/stout/protobuf.hpp f80f5e8adaea82f45c3ab988749076d504bfd509
Diff: https://reviews.apache.org/r/9306/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman
Re: Review Request: Refactored all Try::error and Result::error to use
Error or ErrnoError (also changed a few CHECKs into CHECK_SOMEs).
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On Feb. 8, 2013, 8:21 p.m., Vinod Kone wrote:
> > src/linux/cgroups.cpp, line 99
> > <https://reviews.apache.org/r/9306/diff/1-2/?file=255497#file255497line99>
> >
> > s/any/anything/?
Done.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9306/#review16368
-----------------------------------------------------------
On Feb. 20, 2013, 10:35 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9306/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2013, 10:35 p.m.)
>
>
> Review request for mesos, Vinod Kone and Ben Mahler.
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/common/values.cpp 8aebc0b57de110e14c55c5b6d543f17165cfd397
> src/detector/detector.cpp ca921151a1fc1a57d265b3439d07a8c45e920f21
> src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813
> src/flags/parse.hpp df7e3480c1f5422d185c86cc5f34e7b87b024a2f
> src/linux/cgroups.cpp 03b31e7309b9dd65f00d3b0da2abb81ddaaeea43
> src/linux/fs.cpp ea74c3b935dc716fd53827c8a7eb7010961e7aa6
> src/linux/proc.cpp 8a0fc48dc9769df35d682ece477246b2df2fc0d4
> src/log/coordinator.cpp d9f67bd8b07e6f8b3e022c21f1764d298e314594
> src/log/log.hpp 151eebd5d665993dfaac399553a9a315c093762b
> src/log/replica.cpp 392cb15991657d8e79637e1c4bc21ec813e3117d
> src/logging/logging.cpp e9d778f840f2d6713dbb99d0a8844033be9f0e5f
> src/master/frameworks_manager.cpp 189a7e3b4811643baf285ad29f939453257c705f
> src/sched/sched.cpp f1eeab6f12ee300d77013c6a4ba62ccd7fdb0d1d
> src/slave/cgroups_isolation_module.cpp 14f549edaf1b37a6bca8f75309864333ae775e7c
> src/state/leveldb.cpp 6137b5d089dc9ea4b60fd56153b72e14691df3d8
> src/state/serializer.hpp 1915cefb102526fbad330b090b8c788bea65ed84
> src/state/zookeeper.cpp 4c684797791bb2158d31573d2d47b0da70283bc6
> src/tests/master_tests.cpp 948ab5dff34eeba1f3ce593a864ddf282c8b19ed
> src/tests/stout_tests.cpp fffef3e42774e9b090a098a219a92e642d303d27
> src/zookeeper/group.cpp e74538e3dfa6951178f3e4b49dfd94374a0aa28c
> src/zookeeper/url.hpp c62539b30503a94ed82e11fe84d988c122d36bb6
> third_party/libprocess/include/process/http.hpp 40248dab3df59b322ad9c5a4858961ef471779fd
> third_party/libprocess/include/stout/duration.hpp caf7ce42b34dfb74206d1f10aafabaaec47e7dfc
> third_party/libprocess/include/stout/error.hpp PRE-CREATION
> third_party/libprocess/include/stout/format.hpp fc349903d31d6e31a8147e3a6be87b37698f28f0
> third_party/libprocess/include/stout/fs.hpp 9e62a1b91bc9fac092818ffb3c8bcec46b0bd26d
> third_party/libprocess/include/stout/gzip.hpp 6ff1288683a0d3b05098c772945cc2a8b73f8b01
> third_party/libprocess/include/stout/net.hpp 1909046017da005a5e7c64d525bb3c741d5ad436
> third_party/libprocess/include/stout/numify.hpp 2a59c9269902541d82ba86a2d159984f5bbc6b73
> third_party/libprocess/include/stout/os.hpp 712fb209061384b8c045875ef8898a6efd778514
> third_party/libprocess/include/stout/protobuf.hpp f80f5e8adaea82f45c3ab988749076d504bfd509
>
> Diff: https://reviews.apache.org/r/9306/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request: Refactored all Try::error and Result::error to use
Error or ErrnoError (also changed a few CHECKs into CHECK_SOMEs).
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9306/#review16368
-----------------------------------------------------------
Ship it!
src/linux/cgroups.cpp
<https://reviews.apache.org/r/9306/#comment34836>
s/any/anything/?
src/linux/cgroups.cpp
<https://reviews.apache.org/r/9306/#comment34837>
s/does/do/
- Vinod Kone
On Feb. 8, 2013, 5:49 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9306/
> -----------------------------------------------------------
>
> (Updated Feb. 8, 2013, 5:49 a.m.)
>
>
> Review request for mesos, Vinod Kone and Ben Mahler.
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/common/values.cpp 8aebc0b57de110e14c55c5b6d543f17165cfd397
> src/detector/detector.cpp ca921151a1fc1a57d265b3439d07a8c45e920f21
> src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813
> src/flags/parse.hpp df7e3480c1f5422d185c86cc5f34e7b87b024a2f
> src/linux/cgroups.cpp 03b31e7309b9dd65f00d3b0da2abb81ddaaeea43
> src/linux/fs.cpp ea74c3b935dc716fd53827c8a7eb7010961e7aa6
> src/linux/proc.cpp 8a0fc48dc9769df35d682ece477246b2df2fc0d4
> src/log/coordinator.cpp d9f67bd8b07e6f8b3e022c21f1764d298e314594
> src/log/log.hpp 151eebd5d665993dfaac399553a9a315c093762b
> src/log/replica.cpp 392cb15991657d8e79637e1c4bc21ec813e3117d
> src/logging/logging.cpp e9d778f840f2d6713dbb99d0a8844033be9f0e5f
> src/master/frameworks_manager.cpp 189a7e3b4811643baf285ad29f939453257c705f
> src/sched/sched.cpp f1eeab6f12ee300d77013c6a4ba62ccd7fdb0d1d
> src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36
> src/state/leveldb.cpp 6137b5d089dc9ea4b60fd56153b72e14691df3d8
> src/state/serializer.hpp 1915cefb102526fbad330b090b8c788bea65ed84
> src/state/zookeeper.cpp 4c684797791bb2158d31573d2d47b0da70283bc6
> src/tests/master_tests.cpp 948ab5dff34eeba1f3ce593a864ddf282c8b19ed
> src/tests/stout_tests.cpp fffef3e42774e9b090a098a219a92e642d303d27
> src/zookeeper/group.cpp e74538e3dfa6951178f3e4b49dfd94374a0aa28c
> src/zookeeper/url.hpp c62539b30503a94ed82e11fe84d988c122d36bb6
> third_party/libprocess/include/process/http.hpp 40248dab3df59b322ad9c5a4858961ef471779fd
> third_party/libprocess/include/stout/duration.hpp caf7ce42b34dfb74206d1f10aafabaaec47e7dfc
> third_party/libprocess/include/stout/error.hpp PRE-CREATION
> third_party/libprocess/include/stout/format.hpp fc349903d31d6e31a8147e3a6be87b37698f28f0
> third_party/libprocess/include/stout/fs.hpp 9e62a1b91bc9fac092818ffb3c8bcec46b0bd26d
> third_party/libprocess/include/stout/gzip.hpp 6ff1288683a0d3b05098c772945cc2a8b73f8b01
> third_party/libprocess/include/stout/net.hpp 1909046017da005a5e7c64d525bb3c741d5ad436
> third_party/libprocess/include/stout/numify.hpp 2a59c9269902541d82ba86a2d159984f5bbc6b73
> third_party/libprocess/include/stout/os.hpp 712fb209061384b8c045875ef8898a6efd778514
> third_party/libprocess/include/stout/protobuf.hpp 25d4634bbc51d14ea78c1a922353f0ce52e458df
>
> Diff: https://reviews.apache.org/r/9306/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request: Refactored all Try::error and Result::error to use
Error or ErrnoError (also changed a few CHECKs into CHECK_SOMEs).
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9306/
-----------------------------------------------------------
(Updated Feb. 8, 2013, 5:49 a.m.)
Review request for mesos, Vinod Kone and Ben Mahler.
Changes
-------
Addressed comments.
Description
-------
See summary.
Diffs (updated)
-----
src/common/values.cpp 8aebc0b57de110e14c55c5b6d543f17165cfd397
src/detector/detector.cpp ca921151a1fc1a57d265b3439d07a8c45e920f21
src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813
src/flags/parse.hpp df7e3480c1f5422d185c86cc5f34e7b87b024a2f
src/linux/cgroups.cpp 03b31e7309b9dd65f00d3b0da2abb81ddaaeea43
src/linux/fs.cpp ea74c3b935dc716fd53827c8a7eb7010961e7aa6
src/linux/proc.cpp 8a0fc48dc9769df35d682ece477246b2df2fc0d4
src/log/coordinator.cpp d9f67bd8b07e6f8b3e022c21f1764d298e314594
src/log/log.hpp 151eebd5d665993dfaac399553a9a315c093762b
src/log/replica.cpp 392cb15991657d8e79637e1c4bc21ec813e3117d
src/logging/logging.cpp e9d778f840f2d6713dbb99d0a8844033be9f0e5f
src/master/frameworks_manager.cpp 189a7e3b4811643baf285ad29f939453257c705f
src/sched/sched.cpp f1eeab6f12ee300d77013c6a4ba62ccd7fdb0d1d
src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36
src/state/leveldb.cpp 6137b5d089dc9ea4b60fd56153b72e14691df3d8
src/state/serializer.hpp 1915cefb102526fbad330b090b8c788bea65ed84
src/state/zookeeper.cpp 4c684797791bb2158d31573d2d47b0da70283bc6
src/tests/master_tests.cpp 948ab5dff34eeba1f3ce593a864ddf282c8b19ed
src/tests/stout_tests.cpp fffef3e42774e9b090a098a219a92e642d303d27
src/zookeeper/group.cpp e74538e3dfa6951178f3e4b49dfd94374a0aa28c
src/zookeeper/url.hpp c62539b30503a94ed82e11fe84d988c122d36bb6
third_party/libprocess/include/process/http.hpp 40248dab3df59b322ad9c5a4858961ef471779fd
third_party/libprocess/include/stout/duration.hpp caf7ce42b34dfb74206d1f10aafabaaec47e7dfc
third_party/libprocess/include/stout/error.hpp PRE-CREATION
third_party/libprocess/include/stout/format.hpp fc349903d31d6e31a8147e3a6be87b37698f28f0
third_party/libprocess/include/stout/fs.hpp 9e62a1b91bc9fac092818ffb3c8bcec46b0bd26d
third_party/libprocess/include/stout/gzip.hpp 6ff1288683a0d3b05098c772945cc2a8b73f8b01
third_party/libprocess/include/stout/net.hpp 1909046017da005a5e7c64d525bb3c741d5ad436
third_party/libprocess/include/stout/numify.hpp 2a59c9269902541d82ba86a2d159984f5bbc6b73
third_party/libprocess/include/stout/os.hpp 712fb209061384b8c045875ef8898a6efd778514
third_party/libprocess/include/stout/protobuf.hpp 25d4634bbc51d14ea78c1a922353f0ce52e458df
Diff: https://reviews.apache.org/r/9306/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman
Re: Review Request: Refactored all Try::error and Result::error to use
Error or ErrnoError (also changed a few CHECKs into CHECK_SOMEs).
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On Feb. 5, 2013, 11:18 p.m., Vinod Kone wrote:
> > src/flags/parse.hpp, line 22
> > <https://reviews.apache.org/r/9306/diff/1/?file=255496#file255496line22>
> >
> > ditto, can you include the 'value' here?
This is the parse function, so this should say why parsing failed, not that it failed to parse. Other functions which use this function (see flags/loader.hpp) output the value in their error and then concatenate this error. This is the model we want to follow for error messages because it composes nicely!
> On Feb. 5, 2013, 11:18 p.m., Vinod Kone wrote:
> > src/flags/parse.hpp, line 43
> > <https://reviews.apache.org/r/9306/diff/1/?file=255496#file255496line43>
> >
> > ditto, include the value
See above.
> On Feb. 5, 2013, 11:18 p.m., Vinod Kone wrote:
> > third_party/libprocess/include/stout/os.hpp, lines 958-960
> > <https://reviews.apache.org/r/9306/diff/1/?file=255521#file255521line958>
> >
> > I've seen this code block innumerable times.
> >
> > if (something.isError()) {
> > return Error(something.error());
> > }
> >
> > Makes me think, we could define and use a macro to perform this. Thoughts?
I agree, I just don't know how to do this in a type safe way.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9306/#review16145
-----------------------------------------------------------
On Feb. 8, 2013, 5:49 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9306/
> -----------------------------------------------------------
>
> (Updated Feb. 8, 2013, 5:49 a.m.)
>
>
> Review request for mesos, Vinod Kone and Ben Mahler.
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/common/values.cpp 8aebc0b57de110e14c55c5b6d543f17165cfd397
> src/detector/detector.cpp ca921151a1fc1a57d265b3439d07a8c45e920f21
> src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813
> src/flags/parse.hpp df7e3480c1f5422d185c86cc5f34e7b87b024a2f
> src/linux/cgroups.cpp 03b31e7309b9dd65f00d3b0da2abb81ddaaeea43
> src/linux/fs.cpp ea74c3b935dc716fd53827c8a7eb7010961e7aa6
> src/linux/proc.cpp 8a0fc48dc9769df35d682ece477246b2df2fc0d4
> src/log/coordinator.cpp d9f67bd8b07e6f8b3e022c21f1764d298e314594
> src/log/log.hpp 151eebd5d665993dfaac399553a9a315c093762b
> src/log/replica.cpp 392cb15991657d8e79637e1c4bc21ec813e3117d
> src/logging/logging.cpp e9d778f840f2d6713dbb99d0a8844033be9f0e5f
> src/master/frameworks_manager.cpp 189a7e3b4811643baf285ad29f939453257c705f
> src/sched/sched.cpp f1eeab6f12ee300d77013c6a4ba62ccd7fdb0d1d
> src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36
> src/state/leveldb.cpp 6137b5d089dc9ea4b60fd56153b72e14691df3d8
> src/state/serializer.hpp 1915cefb102526fbad330b090b8c788bea65ed84
> src/state/zookeeper.cpp 4c684797791bb2158d31573d2d47b0da70283bc6
> src/tests/master_tests.cpp 948ab5dff34eeba1f3ce593a864ddf282c8b19ed
> src/tests/stout_tests.cpp fffef3e42774e9b090a098a219a92e642d303d27
> src/zookeeper/group.cpp e74538e3dfa6951178f3e4b49dfd94374a0aa28c
> src/zookeeper/url.hpp c62539b30503a94ed82e11fe84d988c122d36bb6
> third_party/libprocess/include/process/http.hpp 40248dab3df59b322ad9c5a4858961ef471779fd
> third_party/libprocess/include/stout/duration.hpp caf7ce42b34dfb74206d1f10aafabaaec47e7dfc
> third_party/libprocess/include/stout/error.hpp PRE-CREATION
> third_party/libprocess/include/stout/format.hpp fc349903d31d6e31a8147e3a6be87b37698f28f0
> third_party/libprocess/include/stout/fs.hpp 9e62a1b91bc9fac092818ffb3c8bcec46b0bd26d
> third_party/libprocess/include/stout/gzip.hpp 6ff1288683a0d3b05098c772945cc2a8b73f8b01
> third_party/libprocess/include/stout/net.hpp 1909046017da005a5e7c64d525bb3c741d5ad436
> third_party/libprocess/include/stout/numify.hpp 2a59c9269902541d82ba86a2d159984f5bbc6b73
> third_party/libprocess/include/stout/os.hpp 712fb209061384b8c045875ef8898a6efd778514
> third_party/libprocess/include/stout/protobuf.hpp 25d4634bbc51d14ea78c1a922353f0ce52e458df
>
> Diff: https://reviews.apache.org/r/9306/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request: Refactored all Try::error and Result::error to use
Error or ErrnoError (also changed a few CHECKs into CHECK_SOMEs).
Posted by Vinod Kone <vi...@gmail.com>.
> On Feb. 5, 2013, 11:18 p.m., Vinod Kone wrote:
> >
I think its better for us to formalize on the use of single quotes. Currently, we use it inconsistently across the code base.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9306/#review16145
-----------------------------------------------------------
On Feb. 5, 2013, 5:57 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9306/
> -----------------------------------------------------------
>
> (Updated Feb. 5, 2013, 5:57 a.m.)
>
>
> Review request for mesos, Vinod Kone and Ben Mahler.
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/common/values.cpp 8aebc0b57de110e14c55c5b6d543f17165cfd397
> src/detector/detector.cpp ca921151a1fc1a57d265b3439d07a8c45e920f21
> src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813
> src/flags/parse.hpp df7e3480c1f5422d185c86cc5f34e7b87b024a2f
> src/linux/cgroups.cpp 03b31e7309b9dd65f00d3b0da2abb81ddaaeea43
> src/linux/fs.cpp ea74c3b935dc716fd53827c8a7eb7010961e7aa6
> src/linux/proc.cpp 8a0fc48dc9769df35d682ece477246b2df2fc0d4
> src/log/coordinator.cpp d9f67bd8b07e6f8b3e022c21f1764d298e314594
> src/log/log.hpp 151eebd5d665993dfaac399553a9a315c093762b
> src/log/replica.cpp 392cb15991657d8e79637e1c4bc21ec813e3117d
> src/logging/logging.cpp e9d778f840f2d6713dbb99d0a8844033be9f0e5f
> src/master/frameworks_manager.cpp 189a7e3b4811643baf285ad29f939453257c705f
> src/sched/sched.cpp f1eeab6f12ee300d77013c6a4ba62ccd7fdb0d1d
> src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36
> src/state/leveldb.cpp 6137b5d089dc9ea4b60fd56153b72e14691df3d8
> src/state/serializer.hpp 1915cefb102526fbad330b090b8c788bea65ed84
> src/state/zookeeper.cpp 4c684797791bb2158d31573d2d47b0da70283bc6
> src/tests/master_tests.cpp 948ab5dff34eeba1f3ce593a864ddf282c8b19ed
> src/zookeeper/group.cpp e74538e3dfa6951178f3e4b49dfd94374a0aa28c
> src/zookeeper/url.hpp c62539b30503a94ed82e11fe84d988c122d36bb6
> third_party/libprocess/include/process/http.hpp 40248dab3df59b322ad9c5a4858961ef471779fd
> third_party/libprocess/include/stout/duration.hpp caf7ce42b34dfb74206d1f10aafabaaec47e7dfc
> third_party/libprocess/include/stout/error.hpp PRE-CREATION
> third_party/libprocess/include/stout/format.hpp fc349903d31d6e31a8147e3a6be87b37698f28f0
> third_party/libprocess/include/stout/fs.hpp 9e62a1b91bc9fac092818ffb3c8bcec46b0bd26d
> third_party/libprocess/include/stout/gzip.hpp 6ff1288683a0d3b05098c772945cc2a8b73f8b01
> third_party/libprocess/include/stout/net.hpp 1909046017da005a5e7c64d525bb3c741d5ad436
> third_party/libprocess/include/stout/numify.hpp 2a59c9269902541d82ba86a2d159984f5bbc6b73
> third_party/libprocess/include/stout/os.hpp 712fb209061384b8c045875ef8898a6efd778514
> third_party/libprocess/include/stout/protobuf.hpp 25d4634bbc51d14ea78c1a922353f0ce52e458df
>
> Diff: https://reviews.apache.org/r/9306/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request: Refactored all Try::error and Result::error to use
Error or ErrnoError (also changed a few CHECKs into CHECK_SOMEs).
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9306/#review16145
-----------------------------------------------------------
src/files/files.cpp
<https://reviews.apache.org/r/9306/#comment34550>
s/path"/path" + path/
src/files/files.cpp
<https://reviews.apache.org/r/9306/#comment34549>
s/path/path " + result.get() + "/
src/flags/parse.hpp
<https://reviews.apache.org/r/9306/#comment34551>
ditto, can you include the 'value' here?
src/flags/parse.hpp
<https://reviews.apache.org/r/9306/#comment34552>
ditto, include the value
src/linux/cgroups.cpp
<https://reviews.apache.org/r/9306/#comment34553>
thank you
src/linux/cgroups.cpp
<https://reviews.apache.org/r/9306/#comment34554>
include 'hierarchy' value in the message
src/linux/fs.cpp
<https://reviews.apache.org/r/9306/#comment34555>
assuming it fits within 80 chars, can you break after "=" instead?
third_party/libprocess/include/stout/os.hpp
<https://reviews.apache.org/r/9306/#comment34557>
I've seen this code block innumerable times.
if (something.isError()) {
return Error(something.error());
}
Makes me think, we could define and use a macro to perform this. Thoughts?
third_party/libprocess/include/stout/os.hpp
<https://reviews.apache.org/r/9306/#comment34556>
s/release/release number/?
- Vinod Kone
On Feb. 5, 2013, 5:57 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9306/
> -----------------------------------------------------------
>
> (Updated Feb. 5, 2013, 5:57 a.m.)
>
>
> Review request for mesos, Vinod Kone and Ben Mahler.
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/common/values.cpp 8aebc0b57de110e14c55c5b6d543f17165cfd397
> src/detector/detector.cpp ca921151a1fc1a57d265b3439d07a8c45e920f21
> src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813
> src/flags/parse.hpp df7e3480c1f5422d185c86cc5f34e7b87b024a2f
> src/linux/cgroups.cpp 03b31e7309b9dd65f00d3b0da2abb81ddaaeea43
> src/linux/fs.cpp ea74c3b935dc716fd53827c8a7eb7010961e7aa6
> src/linux/proc.cpp 8a0fc48dc9769df35d682ece477246b2df2fc0d4
> src/log/coordinator.cpp d9f67bd8b07e6f8b3e022c21f1764d298e314594
> src/log/log.hpp 151eebd5d665993dfaac399553a9a315c093762b
> src/log/replica.cpp 392cb15991657d8e79637e1c4bc21ec813e3117d
> src/logging/logging.cpp e9d778f840f2d6713dbb99d0a8844033be9f0e5f
> src/master/frameworks_manager.cpp 189a7e3b4811643baf285ad29f939453257c705f
> src/sched/sched.cpp f1eeab6f12ee300d77013c6a4ba62ccd7fdb0d1d
> src/slave/cgroups_isolation_module.cpp 63cefc33cf34eebb82db5d8448b751be8652fa36
> src/state/leveldb.cpp 6137b5d089dc9ea4b60fd56153b72e14691df3d8
> src/state/serializer.hpp 1915cefb102526fbad330b090b8c788bea65ed84
> src/state/zookeeper.cpp 4c684797791bb2158d31573d2d47b0da70283bc6
> src/tests/master_tests.cpp 948ab5dff34eeba1f3ce593a864ddf282c8b19ed
> src/zookeeper/group.cpp e74538e3dfa6951178f3e4b49dfd94374a0aa28c
> src/zookeeper/url.hpp c62539b30503a94ed82e11fe84d988c122d36bb6
> third_party/libprocess/include/process/http.hpp 40248dab3df59b322ad9c5a4858961ef471779fd
> third_party/libprocess/include/stout/duration.hpp caf7ce42b34dfb74206d1f10aafabaaec47e7dfc
> third_party/libprocess/include/stout/error.hpp PRE-CREATION
> third_party/libprocess/include/stout/format.hpp fc349903d31d6e31a8147e3a6be87b37698f28f0
> third_party/libprocess/include/stout/fs.hpp 9e62a1b91bc9fac092818ffb3c8bcec46b0bd26d
> third_party/libprocess/include/stout/gzip.hpp 6ff1288683a0d3b05098c772945cc2a8b73f8b01
> third_party/libprocess/include/stout/net.hpp 1909046017da005a5e7c64d525bb3c741d5ad436
> third_party/libprocess/include/stout/numify.hpp 2a59c9269902541d82ba86a2d159984f5bbc6b73
> third_party/libprocess/include/stout/os.hpp 712fb209061384b8c045875ef8898a6efd778514
> third_party/libprocess/include/stout/protobuf.hpp 25d4634bbc51d14ea78c1a922353f0ce52e458df
>
> Diff: https://reviews.apache.org/r/9306/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>