Henry Robinson has posted comments on this change. Change subject: IMPALA-4822: Implement dynamic log level changes ......................................................................
Patch Set 13: Code-Review+2 (4 comments) Looks pretty good to me. http://gerrit.cloudera.org:8080/#/c/5792/13/be/src/util/logging-support.cc File be/src/util/logging-support.cc: Line 40 What's the effect of removing these? Where is the VLOG masking done now? http://gerrit.cloudera.org:8080/#/c/5792/13/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: PS13, Line 127: set_glog_url =\ : (self.SET_GLOG_LOGLEVEL_URL + "?glog=foo") nit: one line, no parentheses? http://gerrit.cloudera.org:8080/#/c/5792/13/www/log_level.tmpl File www/log_level.tmpl: PS13, Line 21: error It might make more sense to print the form if there was an error, so that the mistake can be rectified. PS13, Line 25: width: 600px; Why set the width? Does the page look bad if you let it figure out the width from the parent div? -- To view, visit http://gerrit.cloudera.org:8080/5792 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I588418e9bcb0b66d33138baf96207a5a35bfbd63 Gerrit-PatchSet: 13 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-HasComments: Yes
