adds ldap auth plugin #55
Conversation
|
Nice! I'll get this reviewed ASAP |
|
cool, thanks. I have it working pretty well right now in a little test vm. On 14 July 2015 at 10:03, Steven Arcangeli notifications@github.com wrote:
|
| @@ -22,6 +23,8 @@ def includeme(config): | |||
| dotted_name = RemoteAccessBackend | |||
| elif dotted_name == 'sql': | |||
| dotted_name = SQLAccessBackend | |||
| elif dotted_name == 'ldap' and LDAP_ENABLED: | |||
| dotted_name = LDAPAccessBackend | |||
There was a problem hiding this comment.
If ldap isn't installed this will produce an unclear message (something about being unable to import the path). I recommend looking at how the caching backend sets itself up (since it isn't guaranteed to have flywheel installed at runtime). It does a lazy import and prints out a clear error message if it fails.
There was a problem hiding this comment.
this should be clearer once python-ldap is in the requirements.txt
|
Good stuff! Left a few comments. |
|
🔔 hey, any outstanding issues left on this? |
| @@ -28,6 +28,7 @@ | |||
| 'six', | |||
| 'transaction', | |||
| 'zope.sqlalchemy', | |||
| 'python-ldap', | |||
There was a problem hiding this comment.
I actually preferred what you were doing earlier with extras_require. It would be better to not require every installation to install python-ldap
|
Two last small nits. Thanks for putting so much work into this! Fixes #16 |
|
👍 |
No description provided.