Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/24583 )
Change subject: KUDU-3788 an option to use pre-built thirdparty artifacts ...................................................................... Patch Set 12: (6 comments) Thank you for addressing the comments. Almost there. Just a few nits. I am not sure if docker builds are automatically tested by dist-cluster or has to be done explicitly. Would be great if you could verify that everything checks out for docker build. Btw, I am fine with addressing any unforeseen issues there in a separate change. http://gerrit.cloudera.org:8080/#/c/24583/11/thirdparty/prebuilt-utils.sh File thirdparty/prebuilt-utils.sh: http://gerrit.cloudera.org:8080/#/c/24583/11/thirdparty/prebuilt-utils.sh@80 PS11, Line 80: cut -d. -f1 > As of PS5, it's only the major version, correct. That's reflected in the c I may have highlighted wrong place that caused the confusion. I should have highlighted the comment instead. I meant in the comment above at line #76: ++ Detect the host OS version string. Use only major and minor versions, ++ http://gerrit.cloudera.org:8080/#/c/24583/12/thirdparty/prebuilt-utils.sh File thirdparty/prebuilt-utils.sh: http://gerrit.cloudera.org:8080/#/c/24583/12/thirdparty/prebuilt-utils.sh@116 PS12, Line 116: $version_str Should this be in quotes to avoid collapse of newlines to spaces and always end up returning 1? http://gerrit.cloudera.org:8080/#/c/24583/12/thirdparty/prebuilt-utils.sh@253 PS12, Line 253: install_subdir This is unused, can be removed here and in callers. http://gerrit.cloudera.org:8080/#/c/24583/12/thirdparty/prebuilt-utils.sh@381 PS12, Line 381: $tarball s/$tarball/$tarball_name ? http://gerrit.cloudera.org:8080/#/c/24583/12/thirdparty/prebuilt-utils.sh@388 PS12, Line 388: if [ -z "$staging_root" ]; then $staging_root will always be non-empty with '/' lying around even if $component and $TP_STAGING_DIR is empty. Maybe add empty check for both separately if the goal is to report the same? http://gerrit.cloudera.org:8080/#/c/24583/12/thirdparty/prebuilt-utils.sh@413 PS12, Line 413: $tarball s/$tarball/$tarball_name ? -- To view, visit http://gerrit.cloudera.org:8080/24583 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I55b5b99fcbe60eec3f6ae19fa4f2e335f361d143 Gerrit-Change-Number: 24583 Gerrit-PatchSet: 12 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Comment-Date: Thu, 30 Jul 2026 12:17:07 +0000 Gerrit-HasComments: Yes
