Skip to content

Conversation

@dosomder
Copy link
Contributor

@dosomder dosomder commented Sep 9, 2014

Replace ldap with ldap3 because ldap lib is not py3 compatible.
Functionality is the same.

#604

@dosomder dosomder force-pushed the backend/604-py3-ldap3 branch 2 times, most recently from d7fa1bb to 2a1e630 Compare September 9, 2014 12:28
Replace ldap with ldap3 because ldap lib is not py3 compatible.
Functionality is the same.
@dosomder dosomder force-pushed the backend/604-py3-ldap3 branch from 2a1e630 to a6e9138 Compare September 9, 2014 12:36
@dosomder
Copy link
Contributor Author

dosomder commented Sep 9, 2014

python3-ldap can not be downloaded with apt-get so I just used pip install in travis script. Not sure if that satifies you. Also removed the dependency from https://github.com/SpriteLink/NIPAP/blob/master/nipap/debian/control

@plajjan
Copy link
Member

plajjan commented Sep 9, 2014

Haven't read the patch yet but depending on things not available in Debian stable or the current Ubuntu (preferably including LTS) is a problem. I really want us to be as compatible with these distributions as possible and with the latest Debian stable we only depend on things that are in the standard distribution, which is nice.

We could roll our own packages or come up with some elaborate scheme of supporting ldap lib on 2.7 and only using ldap3 for python3, and then we would leave it up to the user to fix the dependencies, through virtualenv or otherwise.

There is a general issue with not many python libraries being available as packages in debian/ubuntu for Python3 too..

@dosomder
Copy link
Contributor Author

dosomder commented Sep 9, 2014

That's disappointing. Let's wait for official packages then.

Edit: At least something is going on here: https://launchpad.net/ubuntu/+source/python-ldap3

@plajjan
Copy link
Member

plajjan commented Sep 10, 2014

Don't get me wrong here, I'm all for getting py3 compatibility and using ldap3 is a rather clear step in that direction. We just need to work out a way to have this thing supported on standard Debian & Ubuntu.

This patch is fairly small and there aren't that many ldap/ldap3 calls, perhaps we can botch it to use ldap or ldap3 depending on environment. OR we just roll ldap3 as .deb and include in our debian repo.

@dosomder
Copy link
Contributor Author

I just noticed that exceptions are not correctly catched when using the with statement (e.g. if ldap server is unavailable, it does not throw AuthError). Normally this shouldn't happen, so maybe it's a problem with ldap3 library. Should I switch to try/except/finally?

@plajjan
Copy link
Member

plajjan commented Feb 2, 2015

I'd like to get this integrated. The one remaining blocker was how to provide an ldap3 debian package. ldap3 packages will become available in jessie which is about to be released soon. Unfortunately, I don't think we can expect everyone to upgrade very soon so instead I've decided that we'll roll our own packages (python-ldap3 and python3-ldap3 respectively) in the meantime. Users of jessie or new versions of Ubuntu will rely on their own packages while users of older distros can use our packages.

There's some other work being done on our LDAP module (get AD working). That will most definitely collide with this patch but as this needs to be rebased already there is no way to avoid at least some work.

Would you mind have a go at this again? :)

@plajjan
Copy link
Member

plajjan commented Apr 13, 2015

@dosomder would you like to update this so that it applies cleanly on master?

@dosomder
Copy link
Contributor Author

Sorry I don't have time anymore to work on this project.

@plajjan
Copy link
Member

plajjan commented Apr 15, 2015

@dosomder ok, I understand. Thank you for your contributions thus far :) I will try to brush off this PR and ready it for merge.

@plajjan plajjan self-assigned this Apr 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants