scragg0x / realms-wiki

Git based wiki inspired by Gollum
http://realms.io
GNU General Public License v2.0
831 stars 90 forks source link

realms-wiki setup can clobber config #76

Closed larsimmisch closed 9 years ago

larsimmisch commented 9 years ago

When I invoke realms-wiki setup and enter a cache type of redis at the prompt, realms-wiki.json will only contain the redis configuration:

{
    "CACHE_REDIS_DB": "0",
    "CACHE_REDIS_HOST": "127.0.0.1",
    "CACHE_REDIS_PASSWORD": null,
    "CACHE_REDIS_PORT": 6379
}

Also, I am not prompted for the redis parameters (which is what I would expect).

In other news, I have a fork over at https://github.com/larsimmisch/realms-wiki that adds an LDAP authentication module (branch ldap). Would you be interested in a pull request?

scragg0x commented 9 years ago

Yeah, that is a bug and it's because I failed to get click to behave like I wanted.

Definitely interested in ldap support. I checked out the branch. A few thoughts I wanted to share and maybe get your feedback. Instead of disabling the auth module based on the DBURI, I was thinking of adding ldapauth to the modules list in the config in place of auth. Modules can be activated in the JSON config, this may require some changes to the config code. Also, ldapauth module specific config values could have a LDAP prefix instead of making using DB_URI. I understand the only need for the DB on a standard install is user credentials but their may come a need in future modules/features that require a DB.

larsimmisch commented 9 years ago

Thanks for your quick response.

I couldn't get click to do what you intended either (looking at the docs, I would expect it to work, maybe it is an omission in click). The way forward would be to write a minimal test case and run it past the click people.

I used the DB_URI because it was there, and it was only used for user stuff. So I hijacked it. I'm not particularly attached to doing it this way, though.

I'd be equally happy having a USER_BACKEND config value with 'ldap' or 'db' and then all LDAP stuff prefixed. I'll update my branch and let you know.

larsimmisch commented 9 years ago

I filed a bug on click after discussing it on #pocoo: https://github.com/mitsuhiko/click/issues/429