-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Addition of environment variable overrides to the configuration #199
Addition of environment variable overrides to the configuration #199
Conversation
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! |
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. |
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 |
You're not the hold-up here. I haven't made the time yet to test this. |
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. |
I've added an issue to capture this idea so we don't lose it. Thank you again! |
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.