Bharath Vissapragada has posted comments on this change. Change subject: IMPALA-4822: Implement dynamic log level changes ......................................................................
Patch Set 13: (4 comments) 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? These come in the way of dynamic log4j dynamic log level changes because the per class severity could be different from the global log level and hence passing the if() check. (severity comes from the logging event) 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? Done 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 t Good point. Done. PS13, Line 25: width: 600px; > Why set the width? Does the page look bad if you let it figure out the widt Yes, it basically takes the full page width and looks awkward. Anyway made it 30% rather than a specific number. Let me know if you disagree. -- 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 <bhara...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Henry Robinson <he...@cloudera.com> Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com> Gerrit-HasComments: Yes
