Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12350 )

Change subject: KUDU-2411: linux: Ship the sasl2 modules with the binary 
artifact
......................................................................


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/12350/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12350/1//COMMIT_MSG@9
PS1, Line 9: Because different Linux distributions put the dynamically-loaded
           : libsasl2 modules into different locations, we ship the libraries 
with
           : the binary artifact so that we can set SASL_PATH to a known good
           : location for them at runtime.
> What happens if we didn't do this? I'm perplexed because the libsasl2 modul
The reason is that the libsasl modules are not guaranteed to be installed in 
the same place across all distributions of Linux (case in point: RHEL vs 
Ubuntu). Added the error message to the commit message.


http://gerrit.cloudera.org:8080/#/c/12350/1/build-support/mini-cluster/relocate_binaries_for_mini_cluster.py
File build-support/mini-cluster/relocate_binaries_for_mini_cluster.py:

http://gerrit.cloudera.org:8080/#/c/12350/1/build-support/mini-cluster/relocate_binaries_for_mini_cluster.py@385
PS1, Line 385:   for path in SASL_MODULE_PATHS:
             :     if os.path.exists(path):
             :       children = os.listdir(path)
             :       for child in children:
             :         if PAT_SASL_LIBPLAIN.search(child):
             :           sasl_path = path
             :           break
> FWIW, I think this would be more robust as something like "$(dirname $(ldd
I like your idea... done.


http://gerrit.cloudera.org:8080/#/c/12350/1/build-support/mini-cluster/relocate_binaries_for_mini_cluster.py@393
PS1, Line 393:   if sasl_path is None:
> Nit: "if not sasl_path" is more Pythonic.
Done


http://gerrit.cloudera.org:8080/#/c/12350/1/build-support/mini-cluster/relocate_binaries_for_mini_cluster.py@397
PS1, Line 397:   dest_dir = os.path.join(config[ARTIFACT_LIB_DIR], 'sasl2')
             :   os.makedirs(dest_dir)
> Should we have called prep_artifact_dirs prior to this?
Good point, done.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I44afa05bcfa1a29a90f58c72ed6b0752014a16eb
Gerrit-Change-Number: 12350
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Brian McDevitt <[email protected]>
Gerrit-Reviewer: Grant Henke <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <[email protected]>
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Comment-Date: Wed, 06 Feb 2019 00:17:53 +0000
Gerrit-HasComments: Yes

Reply via email to