[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Review of python-bonsai



Hi Louis-Philippe,

I think I have addressed all your comments. I have pushed fixes in
a temporary branch. I'll integrate them on debian/master once the review
is finished:

https://salsa.debian.org/python-team/packages/python-bonsai/-/commits/review/

See my replies below:

Louis-Philippe Véronneau, Dec 13, 2021 at 23:07:
> * I'm curious to why you need to set "export LC_ALL = C.UTF-8".

This was a leftover from another package. It is useless.

> 5. d/tests:
>
> I don't have an autopkgtests setup that has machine-level isolation. You
> ran that code and it works?

Yes, here is the build log:

http://files.diabeteman.com/juoghahf0teG3Phi/python-bonsai_1.3.0-1_amd64.build.txt

I tried to reproduce the test environment which is prepared for the
github CI actions. This involves generating a docker container with an
LDAP server installed and configured, running that container with
elevated privileges and run the test suite to talk to that container.
It also does some tc voodoo to force timeouts on the TCP connections
(see .ci/delay.py).

I did not manage to run autopkgtests with containers but since the test
needs to run multiple services, a virtual machine made more sense to me.
Also, the use of tc may be a bit too much for a container (I'm not sure
about the limitations, depending on the host).

Some tests are explicitly disabled because I could not get them to work.
I will ping the upstream developer with this when I get a chance.

> 7. Upstream code
>
> Have you read the upstream code? It's something you should do (and you
> should read all the changes for each new update). Not that you have to
> do a proper security audit, but you should go through the code to be
> sure there's no obvious or dangerous things in there.

I have read most of the python code and it looks well written,
documented and tested. C python extensions code is always rather obscure
but I did not see anything suspicious. I am no expert in libldap2
however. The project has been around for some years now. It looks stable
enough to go in Debian in my opinion.

Thanks a lot for your time!


Reply to: