search

CyberPunkMetalHead / gateio-crypto-trading-bot-binance-announcements-new-coins

This is a crypto trading bot that scans the Binance Annoucements page for new coins, and places trades on Gateio
MIT License
1.22k stars 302 forks source link

Allow configuration via environment variables #17

Closed HebelHuber closed 2 years ago

HebelHuber commented 3 years ago

This PR allows to read config data from environment variables.

This is in preparation for another PR to run inside docker and allow configuration via docker-compose.

Also, this is my first PR ever, so let me know if there are any issues.

HebelHuber commented 3 years ago

Changed it to default to config file while allowing overwriting from environment variables. Raising exceptions on missing config variables.

smuu commented 3 years ago

Good work IMO. Would like to see the configuration via environment variables in the master.

One another idea: As it is planned to containerize the application and configure it via environment variables it might be intersting to have default variables if no config file is present and the environment variable is not set.

HebelHuber commented 3 years ago

...it might be intersting to have default variables if no config file is present and the environment variable is not set.

I also thought about that, would it make sense to read the config.example.yml if there is no proper config file and log a warning?

Another option for running in docker would be to just mount the config files, as that would reduce complexity and maybe be more transparent. That would make the whole environment config a non-issue.

Thoughts?

smuu commented 3 years ago

I also thought about that, would it make sense to read the config.example.yml if there is no proper config file and log a warning?

Not sure about that. It is error-prone then, as the user can mess up. Would hard-code that into the application.

Another option for running in docker would be to just mount the config files, as that would reduce complexity and maybe be more transparent. That would make the whole environment config a non-issue.

Configuring container should be done via the environment and not trough configuration files. You can read about that here: https://12factor.net/config Also, mounting files is error-prone in my experience.

HebelHuber commented 3 years ago

Added default values. This also allows removing of the default config in logger.py

HebelHuber commented 2 years ago

Closing this for now as I'm running it by mounting the config file.