impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bharath Vissapragada (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4822: Implement dynamic log level changes
Date Tue, 28 Feb 2017 01:08:47 GMT
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 <bharathv@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bharathv@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message