use a dotfile (optionally) vs forcing secrets to live in the environment
k0s opened this issue · 19 comments
Currently the openphoto script gets the credentials from rather generically-named environment variables:
https://github.com/openphoto/openphoto-python/blob/master/README.markdown
If you're very often running this program, it is probably better to have these secrets live in a dotfile instead of having a two-step process to run the script (source a script then run openphoto). I would advise looking for a dotfile, e.g. $HOME/.openphoto, which contains the credentials and overriding from the environment if the environment variables exist
I agree that this behaviour isn't ideal - but it's currently how the openphoto-php library (et al) work. Pulling in @jmathai - thoughts?
Definitely agree that dotfiles make more sense. Another enhancement might be to require either a dotfile to exist or a separate flag that specifies the location of a file with secrets. I suggest this so that it's visibly apparent what secrets are being used in the scenario that you've got multiple (i.e. when testing or debugging).
So I'd propose -c, --config for the location of the config file ($HOME/.openphoto by default on unix, I have no idea where such things go in windows). I'm personally fine deprecating the environment variables, but it would break backwards compatability plus I would assume all the other language APIs would have to be changed.
I'm definitely +1 on -c, --config (defaulting to $HOME/.openphoto ).
We can break backwards compatibility since these libraries are really new and we're only talking about changing the command line tool.
Sounds great - then, my next question would be - should we have a common config file format? INI file syntax would probably work best for python & php ... I'd imagine ruby would want YAML. It'd be nice to have a single set of secrets though.
I think having a common config file is a good idea. I am fine with .ini or even just key = value (that is .ini without a section)
++ on a shared config file. Looks like Ruby needs a gem to parse INI files. Looping @Beans0063 in for thoughts on INI parsing in Ruby. That should cover the main libs. The rest can deviate if necessary.
Adding another dependency just to parse the config file would not be desirable.
I am good using the key=value formatting
The article here suggests it's a universal format and has simple parser implementations in a bunch of languages
@Beans0063 Adding the parsing logic to the command line code makes sense. Completely agree on not requiring a dependency to do it.
So we're all in agreement and issues are created :).
The format in http://inthebox.webmin.com/one-config-file-to-rule-them-all works for me. I can't imagine we would want more than key, value pairs in such a file (which is all you effectively have in the environment "dictionary" anyway)
So the format for the config files should be INI files without sections. Quoted strings are optional. All of these should be valid.
# unquoted
consumerKey=680ad5ea14ae42b3a29ca95b69b326
consumerSecret=91ce71e636
token=02db36558883c09cc01aac1ac3d45e
tokenSecret=18506814c6
# quoted
consumerKey="680ad5ea14ae42b3a29ca95b69b326"
consumerSecret="91ce71e636"
token="02db36558883c09cc01aac1ac3d45e"
tokenSecret="18506814c6"
# quoted and unquoted mixed
consumerKey=680ad5ea14ae42b3a29ca95b69b326
consumerSecret="91ce71e636"
token=02db36558883c09cc01aac1ac3d45e
tokenSecret="18506814c6"
Wishlist: for UNIX systems, please use $XDG_CONFIG_HOME/openphoto/config.ini and NOT ~/.openphoto/.
On Linux that'd end up being ~/.config/openphoto/config.ini.
Thanks for helping to end dot file clutter. =)
PR #14 implements this, using the "key=value" format (http://inthebox.webmin.com/one-config-file-to-rule-them-all).
There's been some discussion about whether full .ini syntax is a better way to go, since this would allow multiple identities to be stored in the same file - useful for switching between dev and live environments.
I'm still leaning towards the "key=value" syntax, as this has the advantage of simplicity, particularly if we're planning to use this config format across the various language libraries. Multiple identities can still be easily created using separate config files, and selected like so:
openphoto /photos/list.json
openphoto -c dev.conf /photos/list.json
openphoto -c another_host.conf /photos/list.json
@walkah: let me know what you think, and I'll bring the PR up to date ready for merging.
I'm keen to merge this, but would like to make sure we all agree on the best format:
- "key=value" pairs
- .ini format
There was support for "key=value" in earlier comments, and this is my preference. This has been implemented in PR #14, as per @jmathai's comment #5 (comment).
Ini format was raised in the PR #14 comments, as it would allow multiple identities to be stored in one file. Disadvantages previously noted above are that it requires an additional dependency (or more complex parsing logic), and that it must be implemented consistently across all language libraries (as we'd like one format for all languages).
I vote for simple. Multiple identities just adds complexity for not a ton of gain.
I say merge it.
I think we're converging on the best of all worlds - simple key=value pairs, and the option of multiple identities using multiple config files, if desired.
@walkah raised a good point that we should use the (inbox) configparser library if at all possible - it looks like it can be coaxed to handle un-sectioned key=value pairs. I'll take a look shortly.
@sneakypete81 On a side note, are you doing anything interesting with the API? Always curious. Feel free to reply offline - jaisen (at) jmathai (dot) com
Closed in PR#14