Closed theunraveler closed 7 years ago
Sorry for not getting back to you on this sooner. I guess I just didn't notice the PR come through in my various notifications.
Honestly, I'm not thrilled about the idea of encouraging people to put these values in settings.py
as this sort of thing should never be in one's git repo. In our case, we use python-dotenv to keep our private stuff out of settings.py and therefore out of git.
With that said, if you think that this is really important, I'll merge the pull request provided that you (a) adapt it to prioritise the environment variable over the settings value (just invert what you've got) and (b) provide a simple unit test for what you've got here.
Thanks for the consideration--I see your concern.
I do think it's important to be able to use settings.py
to configure these values. Personally, I'd rather store these keys in my private git repo, which only trusted people have access to, than keep the keys somewhere on my computer or an ephemeral EC2 instance and risk losing them (and the encrypted data!) forever. Having the flexibility is good, in my opinion. If you want, you can still stress the importance of using env vars in documentation, etc.
I've made the change that you suggested. Please let me know if you'd like to discuss it more, or if I can do anything else.
Sorry this took so long to merge, but I was at DjangoCon all week :-)
This PR does 2 things:
PASSWORD
andSALT
to be specified as bytes (instead of giving them as strings, then converting to bytes). This change should also be completely backwards compatible, as specifying them as strings will maintain the same behavior.