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: (1 comment) 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"] > I can certainly do that, but you made me curious: what would you like to us Joe once gave me similar review, and I like it very much, because it gather all possible ways of configuring things in one place. https://gerrit.cloudera.org/c/22336/1/tests/conftest.py#57 Having os.environ check like this, suddenly in the middle of the script, can be surprising for future contributor that need to maintain it. -- 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: Tue, 11 Feb 2025 16:48:35 +0000 Gerrit-HasComments: Yes
