----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/6617/#review10666 -----------------------------------------------------------
third_party/libprocess/include/process/http.hpp <https://reviews.apache.org/r/6617/#comment22934> In libprocess we've actually included everything "explicitely", via <process/future.hpp> and <process/pid.hpp> see other files. Note this was not the case with stout, but we may change that in the future. third_party/libprocess/include/process/http.hpp <https://reviews.apache.org/r/6617/#comment22937> Newline please, and a brief comment explaining what this function does. third_party/libprocess/include/process/http.hpp <https://reviews.apache.org/r/6617/#comment22938> const& third_party/libprocess/include/process/http.hpp <https://reviews.apache.org/r/6617/#comment22935> I'd actually just like to see a "statuses" map, just like process/mime.hpp for mime types. third_party/libprocess/include/process/io.hpp <https://reviews.apache.org/r/6617/#comment22933> This isn't needed (yet). third_party/libprocess/include/stout/os.hpp <https://reviews.apache.org/r/6617/#comment22940> Any reason not to return a Try and capture an error string? third_party/libprocess/include/stout/os.hpp <https://reviews.apache.org/r/6617/#comment22941> Also, we already have 'exists', which takes a bool as a second argument to check whether or not it's a directory. I'm not convinced 'exists' is better than isdir and isfile, but we should be opinionated about how we want to do things, and remove the exists mechanism (at least the checking directory part) if we decide not to use it. third_party/libprocess/include/stout/stringify.hpp <https://reviews.apache.org/r/6617/#comment22932> Doesn't look like this is needed. third_party/libprocess/src/decoder.hpp <https://reviews.apache.org/r/6617/#comment22944> Two suggestions: (1) Factor this out into the http::query namespace as a function called 'parse'. Never know if/when it might be useful to others, and to me it "fits" nicely there (very independent piece of code, needs a nice place to live). (2) Return the hashmap rather than take the pointer result and let the compiler do the optimization (eliding copies, etc). In the future we can be even more explicit with C++11 move semantics without having to change the structure (and in many cases syntax) of our code. third_party/libprocess/src/decoder.hpp <https://reviews.apache.org/r/6617/#comment22947> Is there a way to capture that this function is not expected to get called in the parser? third_party/libprocess/src/decoder.hpp <https://reviews.apache.org/r/6617/#comment22946> This is a copy-paste thing, but please go through and remove everything that isn't needed. third_party/libprocess/src/process.cpp <https://reviews.apache.org/r/6617/#comment22945> We should be able to implement this directly in terms of the existing event loop. Let's pair on that together. - Benjamin Hindman On Aug. 22, 2012, 1:37 a.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/6617/ > ----------------------------------------------------------- > > (Updated Aug. 22, 2012, 1:37 a.m.) > > > Review request for mesos, Benjamin Hindman, Vinod Kone, and Jie Yu. > > > Description > ------- > > Implementing the file abstraction and http endpoints for file reading / > browsing. > > > This addresses bug MESOS-255. > https://issues.apache.org/jira/browse/MESOS-255 > > > Diffs > ----- > > src/Makefile.am aaceee3 > src/files/files.hpp d0cab91 > src/files/files.cpp d4080d4 > src/logging/logging.cpp 6909b0b > src/master/http.cpp c480bc6 > src/master/slaves_manager.cpp e25efd0 > src/slave/http.cpp a1f7926 > src/tests/configurator_tests.cpp c2f5aa0 > src/tests/files_tests.cpp PRE-CREATION > src/tests/utils.hpp 83d5daa > src/webui/master/static/controllers.js 1606e64 > third_party/libprocess/include/process/http.hpp 8424ca6 > third_party/libprocess/include/process/io.hpp 6a40b18 > third_party/libprocess/include/stout/hashmap.hpp 51bdea0 > third_party/libprocess/include/stout/json.hpp 25dbcf4 > third_party/libprocess/include/stout/os.hpp b1eceb3 > third_party/libprocess/include/stout/path.hpp 80d9bc6 > third_party/libprocess/include/stout/stringify.hpp ad2f2fa > third_party/libprocess/src/decoder.hpp 105fe5d > third_party/libprocess/src/encoder.hpp ac4886e > third_party/libprocess/src/process.cpp 78069bf > third_party/libprocess/src/statistics.cpp d05b327 > > Diff: https://reviews.apache.org/r/6617/diff/ > > > Testing > ------- > > Added files_tests.cpp > make check > > > Thanks, > > Ben Mahler > >
