Add a logger and update the config handling #85
Conversation
Cadair
force-pushed
the
logging
branch
2 times, most recently
from
6c2fd29
to
f154a71
Compare
baldrick/github/github_api.py
Outdated
| if not branch: | ||
| branch = 'master' | ||
| def get_file_contents(self, path_to_file, branch='master'): | ||
| global file_cache |
There was a problem hiding this comment.
Why the global? You don't need it since you are modifying it inplace
There was a problem hiding this comment.
Explicit is better than implicit? 😀
There was a problem hiding this comment.
Well I still prefer to avoid this if possible and would prefer to use e.g. uppercase notation, but I'll leave it up to you
Codecov Report
@@ Coverage Diff @@
## master #85 +/- ##
=========================================
Coverage ? 82.06%
=========================================
Files ? 17
Lines ? 864
Branches ? 0
=========================================
Hits ? 709
Misses ? 155
Partials ? 0
Continue to review full report at Codecov.
|
baldrick/github/github_api.py
Outdated
| if not branch: | ||
| branch = 'master' | ||
| def get_file_contents(self, path_to_file, branch='master'): | ||
| global file_cache |
There was a problem hiding this comment.
Well I still prefer to avoid this if possible and would prefer to use e.g. uppercase notation, but I'll leave it up to you
|
@astrofrog I have changed it to |
There was a problem hiding this comment.
As long as it works for you, looks fine by me. I'll defer to @astrofrog for final approval.
And I guess the pretty logs are not for the eyes with Heroku admin access?
I was having some issues with my config on Giles, so this is a two pronged attempt at fixing it.
Firstly, this adds
loguruas a nice and easy way for us to do proper logging in Baldrick, which will hopefully make debugging things easier in the future. I had to do some trickery with redirecting the loguru logs back to python stdlib logging for pytest, but it seems to be working fine.Secondly, I have changed the way we cache the config for a repo/branch. Previously we were caching the config keys after all the files had been read. I have changed this so that we cache all github file accesses for one minute. In my opinion this moves the cache to where the requests we want to avoid getting rate limited on and makes the flow of the config functions easier to understand.
I have also tried to make the ordering of the config files more explicit, the three different configs are read in the following order:
bot_nameas the keyI have also made it log at trace and debug the configs it reads from where to make debugging misconfigurations possible.