Henry Robinson has posted comments on this change. Change subject: IMPALA-4057:fix webserver_interface with 127.0.0.1 when start impala process ......................................................................
Patch Set 1: (6 comments) Sorry for the delay - looks pretty good, just a few small comments. http://gerrit.cloudera.org:8080/#/c/4202/1/be/src/util/webserver-test.cc File be/src/util/webserver-test.cc: PS1, Line 94: LocalTest Maybe call this "DefaultInterfaceTest". http://gerrit.cloudera.org:8080/#/c/4202/1/tests/common/impala_cluster.py File tests/common/impala_cluster.py: Line 207: self.service = ImpaladService(self.hostname, self._get_webserver_interface(default="0.0.0.0"), long line Line 234: StateStoredService(self.hostname, self._get_webserver_interface(default="0.0.0.0"), long line Line 242: self.service = CatalogdService(self.hostname, self._get_webserver_interface(default="0.0.0.0"), long line http://gerrit.cloudera.org:8080/#/c/4202/1/tests/common/impala_service.py File tests/common/impala_service.py: Line 47: if self.webserver_interface == "0.0.0.0": I think this would be more succinct as: interface = self.webserver_interface if self.webserver_interface != "0.0.0.0" else self.hostname and then just have one call to urllib.urlopen Line 214: super(StateStoredService, self).__init__(hostname, webserver_interface, webserver_port) long line, maybe break after __init__( -- To view, visit http://gerrit.cloudera.org:8080/4202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idcc29853b6d5db23b82a7000e0d2b80ac7403f87 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: hewenting <hewenting_...@163.com> Gerrit-Reviewer: Henry Robinson <he...@cloudera.com> Gerrit-HasComments: Yes