----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7123/#review11630 -----------------------------------------------------------
src/files/files.cpp <https://reviews.apache.org/r/7123/#comment25118> Use process::mime::types please. src/files/files.cpp <https://reviews.apache.org/r/7123/#comment25116> Please put '{' on new line. src/files/files.cpp <https://reviews.apache.org/r/7123/#comment25119> s/read/download/ src/files/files.cpp <https://reviews.apache.org/r/7123/#comment25120> s/read/download/ src/files/files.cpp <https://reviews.apache.org/r/7123/#comment25129> Close the file please. src/files/files.cpp <https://reviews.apache.org/r/7123/#comment25132> Need to pass the file descriptor to 'reply' so that you can close it. This is a great example of when you might want to do an .onDiscarded and .onFailed to close the file descriptor followed by a .then (we need C++11 now!). src/tests/files_tests.cpp <https://reviews.apache.org/r/7123/#comment25124> Just like we have EXPECT_RESPONSE_STATUS_WILL_EQ and EXPECT_RESPONSE_BODY_WILL_EQ, I could imagine an EXPECT_RESPONSE_CONTENT_TYPE_WILL_EQ ... or something like 'EXPECT_RESPONSE_HEADER_WILL_EQ(expected, "Content-Type", response)' instead ... see my next comment for why wrapping this in EXPECT_RESPONSE_*_WILL_* might be a good idea. src/tests/files_tests.cpp <https://reviews.apache.org/r/7123/#comment25123> You could use EXPECT_RESPONSE_BODY_WILL_EQ now. Of course, this is pretty simple and easy to ready. The upside to using EXPECT_RESPONSE_BODY_WILL_EQ is really that it makes sure you do an 'await' on the future, so as we move code around in tests you don't have to make sure that this EXPECT_EQ is done after something which has awaited on the future. third_party/libprocess/include/stout/os.hpp <https://reviews.apache.org/r/7123/#comment25133> Please put '{' on newline. - Benjamin Hindman On Sept. 14, 2012, 11:48 p.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7123/ > ----------------------------------------------------------- > > (Updated Sept. 14, 2012, 11:48 p.m.) > > > Review request for mesos, Benjamin Hindman and John Sirois. > > > Description > ------- > > Also added a 100MB file read/download limit. > > > This addresses bug MESOS-275. > https://issues.apache.org/jira/browse/MESOS-275 > > > Diffs > ----- > > src/files/files.cpp 806aa35 > src/tests/files_tests.cpp 6ef2004 > third_party/libprocess/include/stout/os.hpp d08b0cb > > Diff: https://reviews.apache.org/r/7123/diff/ > > > Testing > ------- > > added download test > > > Thanks, > > Ben Mahler > >
