David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/15623 )
Change subject: IMPALA-9629: Add CentOS 8.1 support to bootstrap_system.sh ...................................................................... Patch Set 12: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/15623/5/bin/bootstrap_system.sh File bin/bootstrap_system.sh: http://gerrit.cloudera.org:8080/#/c/15623/5/bin/bootstrap_system.sh@266 PS5, Line 266: # 1. Link pip's version to Python's version > Filed IMPALA-9627 for the utility scripts Sorry that I'm slow to get back to this. I definitely won't to hold up this review on this item, but I'm still leery of this change. (Because unless I'm misunderstanding, this is a persistent system-level change, right?) For what it's worth, I think anything in the impala-shell subtree can be ignored, because the real entry point in a dev environment is impala-shell.sh, specifically https://github.com/apache/impala/blob/master/bin/impala-shell.sh#L23. We also have an impala-pip wrapper script in bin/ (which doesn't seem to get much use). This is the first platform where we've had to deal with where python2 wasn't the default. I guess my hand-wavey preference, after setting up the impala python env with the toolchain python/pip as a first step, would be to then always have internal python util scripts defer to that python, rather than assuming the default python is the right version -- regardless of python2 or python3 compatibility. Maybe that's naive though? I might be glossing over difficulties. I think in the interest of progress, I accept your point that the current approach in this patch is a passable interim solution. -- To view, visit http://gerrit.cloudera.org:8080/15623 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3df5d48eca7a10219264e3604a4f05f072188e6e Gerrit-Change-Number: 15623 Gerrit-PatchSet: 12 Gerrit-Owner: Laszlo Gaal <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Laszlo Gaal <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Tue, 14 Apr 2020 18:21:22 +0000 Gerrit-HasComments: Yes
