Alexey Serbin has posted comments on this change. Change subject: KUDU-2005: actionable error messages from webserver ......................................................................
Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/6848/1/src/kudu/server/webserver.cc File src/kudu/server/webserver.cc: PS1, Line 246: TryRunLsof(addr); > Put this after the error message below, since the below puts it in context. I'm not sure I understand -- the error message below is generated from squeasel error message and the http_address_. TryRunLsof() just outputs its messages into the log, since the second argument of TryRunLsof() is empty. Also, that output goes into the WARNING log file, not ERROR one. I looked into the squeasel source code and I could not find the way to detect that condition if not parsing the error message returned from it. That's the excerpt: cry(fc(ctx), "%s: cannot bind to %.*s: %d (%s)", __func__, (int) vec.len, vec.ptr, ERRNO, strerror(errno)); So, I would rather leave it as is for now since I'm not a fan of parsing error messages. Expecting/checking for particular error message patterns in tests is more or less tolerable, but not here. There are too many risks: the squeasel code can change, that particular error message might be not the last one we receive, etc. Let's change the output from TryRunLsof() to remove the ' "Failed to bind to " << addr.ToString() << ". "' part -- that would be less confusion to the readers (I will post that patch separately). -- To view, visit http://gerrit.cloudera.org:8080/6848 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia2315f7ee88c8835a36e5174cb25132967429a77 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-HasComments: Yes
