Marton Greber has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23352 )

Change subject: KUDU-2345 Add developer docs for the python client
......................................................................


Patch Set 2:

(5 comments)

Thank you for working on this Gabi!
Overall looks good to me, just a couple notes/nits.

http://gerrit.cloudera.org:8080/#/c/23352/2/python/README.md
File python/README.md:

http://gerrit.cloudera.org:8080/#/c/23352/2/python/README.md@22
PS2, Line 22:
I just noticed that on some installations we might need to manually run OS 
package manager install for setuptools, and python developer packages:
eg for me:
sudo apt install python3-setuptools
sudo apt install python3-dev.

Then I remembered, that we have this script:
https://github.com/apache/kudu/blob/master/docker/bootstrap-python-env.sh
That aims to do this.

I think you can add a small section that there are some OS packages required to 
build the Python client, eg setuptools, python developer package. 
https://github.com/apache/kudu/blob/master/docker/bootstrap-python-env.sh is a 
script that does this for the supported platforms.


http://gerrit.cloudera.org:8080/#/c/23352/2/python/README.md@24
PS2, Line 24:
maybe its advantageous to add a link to virtualenv website if someone does not 
have it installed:
https://virtualenv.pypa.io/en/latest/index.html


http://gerrit.cloudera.org:8080/#/c/23352/2/python/README.md@71
PS2, Line 71: cd /path/to/kudu/python
nit: its a bit nicer since you already define KUDU_HOME above ->
cd $KUDU_HOME/python


http://gerrit.cloudera.org:8080/#/c/23352/2/python/README.md@74
PS2, Line 74: python setup.py build_ext
nit: i've just checked the after build_ext i would like to run a single 
unittest.
it gives me and error as the *.so files are compiled into the build directory, 
but we need them in the kudu/ directory because this is how the project is 
setup.
to make this happen we need to use
python setup.py build_ext --inplace.
(it is interesting that if we run python setup.py test, under the hood it uses 
--inplace as the compiled *.so files are in the kudu/ directory)


http://gerrit.cloudera.org:8080/#/c/23352/2/python/README.md@88
PS2, Line 88:
maybe add a debugging:
Debugging A Test:
python -m pdb -m unittest kudu.tests.test_client.TestClient.test_list_tables



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd38ecd5acf4a40c7f04d89c073d093ac5c83e0
Gerrit-Change-Number: 23352
Gerrit-PatchSet: 2
Gerrit-Owner: Gabriella Lotz <[email protected]>
Gerrit-Reviewer: Gabriella Lotz <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <[email protected]>
Gerrit-Comment-Date: Sat, 06 Sep 2025 10:55:46 +0000
Gerrit-HasComments: Yes

Reply via email to