> On Aug. 23, 2012, 5:47 a.m., Benjamin Hindman wrote: > > third_party/libprocess/include/process/http.hpp, line 151 > > <https://reviews.apache.org/r/6617/diff/3/?file=143745#file143745line151> > > > > I'd actually just like to see a "statuses" map, just like > > process/mime.hpp for mime types.
done similarly to mime now > On Aug. 23, 2012, 5:47 a.m., Benjamin Hindman wrote: > > third_party/libprocess/include/process/http.hpp, lines 7-8 > > <https://reviews.apache.org/r/6617/diff/3/?file=143745#file143745line7> > > > > 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. done > On Aug. 23, 2012, 5:47 a.m., Benjamin Hindman wrote: > > third_party/libprocess/include/stout/os.hpp, line 88 > > <https://reviews.apache.org/r/6617/diff/3/?file=143749#file143749line88> > > > > Any reason not to return a Try and capture an error string? ok done, I return false on EACCES, and error otherwise > On Aug. 23, 2012, 5:47 a.m., Benjamin Hindman wrote: > > third_party/libprocess/src/decoder.hpp, line 347 > > <https://reviews.apache.org/r/6617/diff/3/?file=143752#file143752line347> > > > > Is there a way to capture that this function is not expected to get > > called in the parser? I suppose I could do an assert if you like? > On Aug. 23, 2012, 5:47 a.m., Benjamin Hindman wrote: > > third_party/libprocess/src/decoder.hpp, line 386 > > <https://reviews.apache.org/r/6617/diff/3/?file=143752#file143752line386> > > > > This is a copy-paste thing, but please go through and remove everything > > that isn't needed. whoops, just query isn't needed > On Aug. 23, 2012, 5:47 a.m., Benjamin Hindman wrote: > > third_party/libprocess/src/decoder.hpp, line 82 > > <https://reviews.apache.org/r/6617/diff/3/?file=143752#file143752line82> > > > > 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. done and done > On Aug. 23, 2012, 5:47 a.m., Benjamin Hindman wrote: > > third_party/libprocess/include/stout/os.hpp, lines 333-339 > > <https://reviews.apache.org/r/6617/diff/3/?file=143749#file143749line333> > > > > 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. ok so I've removed the directory flag in exists (which I thought made exist calls a bit confusing to the unenlightened reader) now, exists is effectively (isfile() || isdir), I cleaned up the directory flag using calls, but I left all other exists calls, shall I wipe those too? They seem to always know whether they want a file or a dir.. just want to double check before I do even more cleanup - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/6617/#review10666 ----------------------------------------------------------- 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 > >
