Bharath Vissapragada has posted comments on this change. Change subject: IMPALA-4822: Implement dynamic log level changes ......................................................................
Patch Set 6: (7 comments) http://gerrit.cloudera.org:8080/#/c/5792/6/be/src/util/logging-support.cc File be/src/util/logging-support.cc: Line 83: static const string log_inputs[] = {"get_java_log", "set_java_log", "reset_java_log", > Sorry, I didn't realize the log_inputs and the display_members were differe Changes undone. Renamed a little as discussed offline. http://gerrit.cloudera.org:8080/#/c/5792/6/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: Line 43: """ Helper method that polls a given url and asserts the return code is ok and > nit: remove space before "Helper" Done Line 48: def test_log_level_callback(self): > test_log_level Done Line 50: malformed inputs""" > Does not test that the log level modifications are actually in effect. Done Line 59: # Set the log level of a class to TRACE anc confirm the setting is in place > typo: and Done Line 67: self.get_and_check_status(get_loglevel_url, "TRACE") > also check a different class and make sure it's still at DEBUG Done Line 75: self.get_and_check_status(get_loglevel_url, "DEBUG") > also add tests for malformed inputs, e.g., a class that does not exist, emp Added a few more tests. Also, per my understanding, in log4j, there is no such thing as a class that does not exists. It accepts any class that we input (and sets a log level), since we can actually load them at runtime, if it is not already loaded. -- 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: 6 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-HasComments: Yes