----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7454/#review12229 -----------------------------------------------------------
None are yours but let's clean this up while we're at it! third_party/libprocess/include/stout/net.hpp <https://reviews.apache.org/r/7454/#comment25954> While we're here, I would wrap this with an additional error string, two options: 1. Wrap the error: "Failed to open file '<path>': " + fd.error() 2. Change os::open to be more descriptive and just have this return fd directly. Not sure about this option since we don't have a consistent error format in os.hpp, some are raw strerror, some are wrapped with more info. third_party/libprocess/include/stout/net.hpp <https://reviews.apache.org/r/7454/#comment25955> I missed this in my Try refactor: Try<Nothing> close = os::close(fd.get()) // Then I guess ignore the close error since we've failed at this point anyway. third_party/libprocess/include/stout/net.hpp <https://reviews.apache.org/r/7454/#comment25956> Handle null FILE* case and fail accordingly. third_party/libprocess/include/stout/net.hpp <https://reviews.apache.org/r/7454/#comment25958> Seems that we should return an error when the response code is anything other than 200 OK. Maybe we return a Try<Nothing>? What do you think? Currently it is not documented what this returns, can you update the doc at the top? - Ben Mahler On Oct. 8, 2012, 5:33 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7454/ > ----------------------------------------------------------- > > (Updated Oct. 8, 2012, 5:33 p.m.) > > > Review request for mesos, Benjamin Hindman, Florian Leibert, and Ben Mahler. > > > Description > ------- > > See summary > > > Diffs > ----- > > src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee > third_party/libprocess/include/stout/net.hpp > f6b770c8ca7d21e0aca0f614168a0985c77046b0 > > Diff: https://reviews.apache.org/r/7454/diff/ > > > Testing > ------- > > Tested locally > > > Thanks, > > Vinod Kone > >
