Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/18939 )

Change subject: IMPALA-10262: RPM/DEB Packaging Support
......................................................................


Patch Set 10:

(5 comments)

Thanks for the feedbacks! Addressed the comments and improved the scripts
 - Add scripts to stop daemons
 - Add waiting logic in start scripts to wait until service available

Also skip packaging impala-shell on redhat8 to avoid the shebang issue.

http://gerrit.cloudera.org:8080/#/c/18939/8/CMakeLists.txt
File CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/18939/8/CMakeLists.txt@614
PS8, Line 614:     # Set a meaningful package name, e.g. 
Impala-4.3.0-SNAPSHOT_hive-3.1.3000.7.2.18.0-41-x86_64.el7
             :     set(CPACK_PACKAGE_FILE_NAME 
"${CPACK_PACKAGE_NAME}-${CPACK_PACKAGE_VERSION}")
             :     set(CPACK_PACKAGE_FILE_NAME 
"${CPACK_PACKAGE_FILE_NAME}_hive-$ENV{IMPALA_HIVE_VERSION}")
             :     set(CPA
> One thing I tried out is using the CPACK_DEBIAN_DEBUGINFO_PACKAGE option. T
Good to know that. We can do it in future changes.


http://gerrit.cloudera.org:8080/#/c/18939/8/CMakeLists.txt@622
PS8, Line 622:     endif()
             :     message(STATUS "Package name: ${CPACK_PACKAGE_FILE_NAME}")
             :
             :     if ($ENV{STRIP_DEPLOYMENT_IMPALAD})
             :       set(CPACK_STRIP_FILES ${IMPALA_INSTALLDIR}/bin/impalad)
             :
> When I run this on Ubuntu 20, somehow this doesn't work. It works on the co
It's strange that I tried Ubuntu 20 on EC2 and it works. Changed it anyway 
since this might be more compatible.


http://gerrit.cloudera.org:8080/#/c/18939/8/CMakeLists.txt@628
PS8, Line 628:     endif()
> As a sanity check, can we raise an error if PKG_LIST is empty:
Sure. Done.


http://gerrit.cloudera.org:8080/#/c/18939/8/docker/install_os_packages.sh
File docker/install_os_packages.sh:

http://gerrit.cloudera.org:8080/#/c/18939/8/docker/install_os_packages.sh@178
PS8, Line 178: woul
> Nit: maybe change to 'would' ?
Done


http://gerrit.cloudera.org:8080/#/c/18939/8/package/bin/impala-env.sh
File package/bin/impala-env.sh:

http://gerrit.cloudera.org:8080/#/c/18939/8/package/bin/impala-env.sh@32
PS8, Line 32:
            : #TODO: Add graceful shutdown for impalads
            : function stop_process {
            :   name=$1
> Good point, it looks like we don't include that right now, and it is needed
I thought the native libs should exist if DataNode is deployed on the node. But 
it's harmless to include them in the package. Done.



--
To view, visit http://gerrit.cloudera.org:8080/18939
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I64419fd400fe8d233dac016b6306157fe9461d82
Gerrit-Change-Number: 18939
Gerrit-PatchSet: 10
Gerrit-Owner: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Laszlo Gaal <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Reviewer: Xiang Yang <[email protected]>
Gerrit-Reviewer: Yifan Zhang <[email protected]>
Gerrit-Comment-Date: Fri, 07 Jul 2023 09:09:50 +0000
Gerrit-HasComments: Yes

Reply via email to