search

magicalraccoon / tootstream

A command line interface for interacting with Mastodon instances
MIT License
259 stars 32 forks source link

Addition of environment variable overrides to the configuration #199

Closed samdmarshall closed 5 years ago

samdmarshall commented 5 years ago

hi, this PR aims to allow users to provide the sensitive information that tootstream needs, in a (potentially) more secure form (session-based environment variables). All of the details are explain in comments in the change-set.

I didn't make changes to the changelog file because it wasn't clear if I should be adding to that or the primary maintainers do.

craigmaloney commented 5 years ago

Thank you for this.

I'm not clear what this solves other than allowing for environment variables to be used for storing configuration Is that the aim of this? (I'm trying to learn as well).

Thanks again!

samdmarshall commented 5 years ago
  1. it was my desire to keep sensitive tokens out of my dot-files, so I can push up my configurations and synchronize them between my systems.
  2. adding the ability to iterate on local development without being tied to real accounts/production environments; eg: spin up mastodon server docker container with a set of creds to use, provided to the client as environment variables, and don't have to worry about these creds getting saved to disk, and get straight to testing work on a particular feature.

tbh so far I love using tootstream and maybe this isn't the right solution to this problem but I didn't feel ready to try to implement a whole system with the configuration file to handle getting info from external programs (eg password/secret managers). I implemented and tested this myself for my own configuration. I hope if this does get merged that someone bangs on it a lot first because I can tell that there could be some peculiar behaviors with this depending on both config and environment variable contents.

samdmarshall commented 5 years ago

just want to ensure my due diligence on this: i’m not holding up this process at all? i am more familiar than most wrt the luxury it is to maintain open source software and life comes first — just figured i should ask to see if i can do anything to make this easier

craigmaloney commented 5 years ago

You're not the hold-up here. I haven't made the time yet to test this.

craigmaloney commented 5 years ago

OK, I gave this a quick peek and I think I'm understanding why I'm reluctant to pull this.

Part of the issue is that I'd like to keep the configuration pieces all in one spot. This moves that around throughout the file (both from getting the environment variables at the top, and then later on getting the instance config at the bottom).

The second thing is that the main conditional for all of the config (if set) is "pass". That's a code smell to me.

I'd rather we did this all in one method that returns the configuration. This means that we're going to have to re-write how the configuration is passed throughout the program. Ideally I'd like to have it so tootstream configurations can be changed on-the-fly. The idea for this will be helpful for adding environment variables for default values, but it'll make the process for on-the-fly config changes a little more difficult as implemented.

Thank you for the suggestion though and for your generous pull request.. I'm going to close this out until we can get a better handle on how to handle configuration in Tootstream.

craigmaloney commented 5 years ago

I've added an issue to capture this idea so we don't lose it. Thank you again!