> On Aug. 16, 2012, 12:12 a.m., Jie Yu wrote: > > src/files/files.hpp, line 26 > > <https://reviews.apache.org/r/6617/diff/1/?file=140337#file140337line26> > > > > newline if two includes do not share the same prefix.
done > On Aug. 16, 2012, 12:12 a.m., Jie Yu wrote: > > src/files/files.cpp, line 439 > > <https://reviews.apache.org/r/6617/diff/1/?file=140338#file140338line439> > > > > Yes, i think you need to delete the 'process' as it is spawned without > > setting manage=true. ok done, I don't see why it has to be allocated with new in the first place? > On Aug. 16, 2012, 12:12 a.m., Jie Yu wrote: > > src/tests/utils.hpp, line 89 > > <https://reviews.apache.org/r/6617/diff/1/?file=140341#file140341line89> > > > > Can we just use mkdir here? The new name looks a little weird to me:) done > On Aug. 16, 2012, 12:12 a.m., Jie Yu wrote: > > src/files/files.cpp, line 230 > > <https://reviews.apache.org/r/6617/diff/1/?file=140338#file140338line230> > > > > Is there any other way to check whether the 'name' is a directory or > > not? This is not quite elegant to me:) I'm taking this out but to be clear, this isn't checking if it's a directory, rather just whether it looks like a directory so that we can avoid unnecessarily resolving the name. The actual directory check happens later using: // Don't read directories. if (os::exists(path.get(), true)) { return NotFound("Cannot read a directory."); } Now though, with os::isdir So since this / check is ugly and not necessary, I've taken it out. > On Aug. 16, 2012, 12:12 a.m., Jie Yu wrote: > > src/files/files.cpp, line 108 > > <https://reviews.apache.org/r/6617/diff/1/?file=140338#file140338line108> > > > > Instead of opening the file here, can we have a new API os::access in > > stout which wraps the libc function "access"? > > Benjamin Hindman wrote: > +1 good call! done. > On Aug. 16, 2012, 12:12 a.m., Jie Yu wrote: > > src/files/files.cpp, line 205 > > <https://reviews.apache.org/r/6617/diff/1/?file=140338#file140338line205> > > > > We can get rid of it once we have the APIs above. done > On Aug. 16, 2012, 12:12 a.m., Jie Yu wrote: > > src/files/files.cpp, line 194 > > <https://reviews.apache.org/r/6617/diff/1/?file=140338#file140338line194> > > > > Can we provide APIs in stout/os that basically have the same semantics > > as that in python APIs like os.path.isdir(path), os.path.isfile(path), etc. done - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/6617/#review10375 ----------------------------------------------------------- On Aug. 17, 2012, 4:15 a.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/6617/ > ----------------------------------------------------------- > > (Updated Aug. 17, 2012, 4:15 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 b0cb6cc > src/files/files.hpp d0cab91 > src/files/files.cpp d4080d4 > src/logging/logging.cpp 6909b0b > src/master/http.cpp e783ac3 > 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 caf5926 > src/webui/master/static/controllers.js 1606e64 > third_party/libprocess/include/process/http.hpp 8424ca6 > 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/statistics.cpp d05b327 > > Diff: https://reviews.apache.org/r/6617/diff/ > > > Testing > ------- > > Added files_tests.cpp > make check > > > Thanks, > > Ben Mahler > >
