search

nginxinc / nginx-ldap-auth

Example of LDAP authentication using ngx_http_auth_request_module
BSD 2-Clause "Simplified" License
686 stars 202 forks source link

Python3 #66

Closed goochjj closed 4 years ago

goochjj commented 5 years ago

Update code and Dockerfile for python 3

vl-homutov commented 5 years ago

Thank you for looking into this. Will this change break things for those who don't have python 3? This is something I'd like to avoid when adding support for python3. To accept this, we need to review currently supported OSes/packages, so it will take some time.

goochjj commented 5 years ago

Yes, the library names changed, as did base64's handling of strings vs bytes.

When I wrote it in my original branch, I named the python3 version "nginx-ldap-auth-daemon3" instead... For the purpose of the PR it made sense to show the differentials. That could be how you support both - have the user select the version of the script for the version of python.

Sadly my python experience is not advanced enough for me to know how to do both simultaneously, but I think that'd just make the code unreadable anyway - python 3 is the future, python2 is deprecated as of next year - clearly python3 is the future, so supporting python 2 should be a secondary goal. If it were me I'd deprecate the python 2 version and give people type to move to the python 3 version, especially given how easy it is to do virtualenv. And the dockerfile should probably be python 3 because why not.... if the dependencies are already encapsulated then 2 vs 3 doesn't matter to the end user.

vl-homutov commented 4 years ago

Python3 support was added. If you feel there is something missing, please create new PR against current master.