zopefoundation / Products.PluggableAuthService

Pluggable Zope authentication / authorization framework
Other
9 stars 18 forks source link

Python 2 / 3 compatible imports. #10

Closed rudaporto closed 6 years ago

rudaporto commented 6 years ago

@pbauer can you help me with this review or should I ask someone else?

rudaporto commented 6 years ago

@tomgross I update with more compatible changes.

Still, there is some unused imports but I did not removed them at this stage.

rudaporto commented 6 years ago

@tomgross should I change travis.yaml to use tox run the build with python 2.7, 3.4, 3.5 and 3.6?

tomgross commented 6 years ago

@rudaporto This package depends on GenericSetup (Python 3 upgrade is WIP) and ZServer (This package will probably never see Python 3)

To get an overview what is achieved it is probably a good idea to have them with allow_failures option on. I don't know, if there is an official statement on Python 3 subversion compatibility but I'd focus on 3.5 and 3.6.

tomgross commented 6 years ago

Maybe it is a good idea to have coveralls run too to see where the blind spots are.

tomgross commented 6 years ago

Apart from that this LGTM.

tseaver commented 6 years ago

We should make the dependency on ZServer optional. The dependency is there to support detection of an FTPRequest, but if ZServer is not installed, we can just create a no-op class, e.g.:

# in Products/PluggableAuthService/plugins/RequestTypeSniffer.py
try:
    from ZServer.FTPRequest import FTPReques
except ImportError:
    class FTPRequest(object):
        """Dummy for missing ZServer's 'FTPRequest'."""
icemac commented 6 years ago

@tseaver +1 to make ZServer optional, some weeks ago I created #8 to address this issue. Maybe it can be done in another PR.

icemac commented 6 years ago

What has still to be done to get this PR merged?