> On Oct. 8, 2012, 6:10 p.m., Ben Mahler wrote: > > third_party/libprocess/include/stout/net.hpp, line 35 > > <https://reviews.apache.org/r/7454/diff/1/?file=174372#file174372line35> > > > > 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.
done and done > On Oct. 8, 2012, 6:10 p.m., Ben Mahler wrote: > > third_party/libprocess/include/stout/net.hpp, line 43 > > <https://reviews.apache.org/r/7454/diff/1/?file=174372#file174372line43> > > > > 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. why do we need to store it in a variable? > On Oct. 8, 2012, 6:10 p.m., Ben Mahler wrote: > > third_party/libprocess/include/stout/net.hpp, line 68 > > <https://reviews.apache.org/r/7454/diff/1/?file=174372#file174372line68> > > > > 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? I think returning the response code is valuable, the way we do it here. The user can decide what he wants to do based on the response. This is documented at top, not sure what you want? - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7454/#review12229 ----------------------------------------------------------- On Oct. 8, 2012, 7:27 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, 7:27 p.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Description > ------- > > Better error messages > > > Fix for curl download > > > Diffs > ----- > > src/launcher/launcher.cpp 5de1f479cf03182a1c12bdd951b5bd57c00ee2ee > third_party/libprocess/include/stout/net.hpp > f6b770c8ca7d21e0aca0f614168a0985c77046b0 > third_party/libprocess/include/stout/os.hpp > 13dbc715ed08cfe6b24ee20744d427dac1104694 > > Diff: https://reviews.apache.org/r/7454/diff/ > > > Testing > ------- > > Tested locally > > > Thanks, > > Vinod Kone > >
