romana / rlog

A simple Golang logger without external dependencies
Apache License 2.0
39 stars 13 forks source link

Enable reloading config from environment #21

Closed muru closed 6 years ago

muru commented 6 years ago

For #2, using the environment is more convenient than creating a temporary file for updating config during runtime. So I extracted out the code from init() and made it public.

CLAassistant commented 6 years ago

CLA assistant check
All committers have signed the CLA.

jbrendel commented 6 years ago

Hello! Thank you for your submission. However, I wonder if/how that works. Generally, the environment of a process cannot be updated after the process has started already. Therefore, re-reading environment variables won't yield any changed results, will they? Or do you suggest that the process somehow changes its own environment variables and then calls UpdateEnv? For example with os.Setenv?

jbrendel commented 6 years ago

Ok, I tested it and it works. It still seems kinda odd that we'd have two calls (one to os.System and one to rlog.UpdateEnv) to accomplish one thing, though. I need to think about this for a bit.

jbrendel commented 6 years ago

I have accepted and merged this PR, updated the examples and docs to reflect your changes. Thank you very much again!

muru commented 6 years ago

Ah, yes, the two calls didn't bother me much since I'd been writing a wrapper around rlog anyway (so I'd have a logger.SetLevel(...) which does os.SetEnv() and rlog.UpdateEnv()). I'd played around with a few ideas:

While trying out the last option, I realized that simply re-using the init() code is the simplest change. If I wanted to update multiple settings at once, then calling a setter multiple times (with the corresponding re-reading of config files, etc.) would be very wasteful, butrlog.UpdateEnv can assimilate all changed settings. All other changes would have been more invasive. 😓