> On Aug. 18, 2015, 6:34 p.m., Vinod Kone wrote: > > src/launcher/fetcher.cpp, lines 100-106 > > <https://reviews.apache.org/r/37584/diff/1/?file=1043203#file1043203line100> > > > > just do > > > > return Error("Skipping fetch with Hadoop client: " + > > (available.isError() ? availabe.error() : " client not > > found")); > > Marco Massenzio wrote: > So, that was my first choice, but I then reflected that my finding out > the site of the error was made more complicated due to the available log > lines being far away from it; so I had to do some digging and investigating. > > In general, I have been taught to prefer to have a LOG(ERROR) near the > site where the error actually happens, as that also avoids running wild goose > chases :) > Especially if it so happens that the error message may be > (unintentionally) misleading. > > What do you think?
I think it's not consistent with how other cases are handled in this file. This particular should've been easy to find because the call site is also in fetcher.cpp. I'll fix this and commit for you. - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37584/#review95754 ----------------------------------------------------------- On Aug. 18, 2015, 5:24 p.m., Marco Massenzio wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37584/ > ----------------------------------------------------------- > > (Updated Aug. 18, 2015, 5:24 p.m.) > > > Review request for mesos and Adam B. > > > Bugs: MESOS-3287 > https://issues.apache.org/jira/browse/MESOS-3287 > > > Repository: mesos > > > Description > ------- > > When trying to download an artifact with the Hadoop client, > and the client is not `available()` we correctly return a > false value, but then in the error message, we tried to > make a call to `Try::error` which failed and crashed Master. > > This fixes it. > > > Diffs > ----- > > src/launcher/fetcher.cpp f8b46b8f957942d0cfdc45d3361f52dae3e514a3 > > Diff: https://reviews.apache.org/r/37584/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Marco Massenzio > >
