Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22438 )
Change subject: IMPALA-13724: Add hostnames for Docker host and gateway to Impala containers ...................................................................... Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/22438/1/bin/start-impala-cluster.py File bin/start-impala-cluster.py: http://gerrit.cloudera.org:8080/#/c/22438/1/bin/start-impala-cluster.py@1021 PS1, Line 1021: internal_listen_host = os.environ["INTERNAL_LISTEN_HOST"] > Oh, I completely agree on how configurability should be set up, that wasn't I have no other intention but to gather the configuration in single place. I realize now that it probably does not make sense to have it parameterized, unless maybe I'm hacking my own docker network setup, in local machine, that has weird IP addresses, rather than through Jenkins. In that case, please just make this a INTERNAL_LISTEN_HOST constant before L58 like this: https://github.com/apache/impala/blob/c9e9f86c0536f0991f70613675bbcfbbccc6a324/tests/common/impala_test_suite.py#L159 Or, place the constant at tests/common/environ.py for reuse. http://gerrit.cloudera.org:8080/#/c/22438/1/bin/start-impala-cluster.py@1044 PS1, Line 1044: + > I'll raise a separate patch for critique-gerrit-review.py about this. I think we agree that we should follow the new W504 recommendation? -- To view, visit http://gerrit.cloudera.org:8080/22438 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I545607c0bb32f8043a0d3f6045710f28a47bab99 Gerrit-Change-Number: 22438 Gerrit-PatchSet: 1 Gerrit-Owner: Laszlo Gaal <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Laszlo Gaal <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Wed, 12 Feb 2025 18:28:19 +0000 Gerrit-HasComments: Yes
