Aldriana / ShadowCraft-Engine

Calculations backend for ShadowCraft, a WoW theorycraft project.
GNU Lesser General Public License v3.0
37 stars 22 forks source link

Logging #57

Closed dazer closed 13 years ago

dazer commented 13 years ago

This is what I was discussing in https://github.com/Aldriana/ShadowCraft-Engine/issues#issue/31. Callers look like log.log_this(log.logger_name, 'level', 'message'); for instance a logging call from AldrianaRogueDamageCalculator would be log.log_this(log.aldriana, 'error', 'Something broke')

log.py is designed in a very programatic way: most of what it does can be written in a config file using code like this if you like that approach better: [loggers] keys=root,log02,log03,log04,log05,log06,log07

[handlers]
keys=hand01,hand02,hand03

[formatters]
keys=form01,form02

[log01]
...

The issue with the user of the library unaware of its logging nature is apparently solved passing the NullHandler (which is a 2.7 feature) to the loggers, so you are logging stuff but not doing anything with it unless the user configures log.py and defines what handlers to use.

The logger names I chose need some review and the whole thing may need some refactor if you guys find it more convenient to declare what logger to use in each module instead of choosing which one to use when passing arguments to log_this or anything else.

It's fully functional as of now, but I'm open to suggestions to improve it: seems to me like it needs a lot.

edit: the imports at __init__ are doing next to nothing (which is expected; I messed that up when testing and had it imported). Hence, log.py as it stands needs to be imported from each module in which we need to logg stuff. Still I figure you get the idea here. Maybe some arangement akin to exceptions would be nice.

Aldriana commented 13 years ago

Sorry for being slow on this - was sort of busy over the weekend and was spending what time I had getting the Sub model written. I'll take a look at this in the next day or two.

dazer commented 13 years ago

No worries about the delay: all in all I doubt this is top priority. Not much of a change in the last commits: I put imports where they'd be used and matched each file with its logger (actually to their aliases). Callers, as shown in test.py don't need to pick a logger since they are passing what's in logger = ... at the top of the file. log.py now only defines the log_this function. We can do without it and use logger.level('message') as a caller instead; that looks 'samplish' enough I think. Then again, I simply don't know if this is what suits our needs.

Note: Since my fork is previous to the change to test.py I have no idea how this would merge but I can pull a new request if it causes any conflict.

Aldriana commented 13 years ago

What's with the empty (binary) files checked in?

dazer commented 13 years ago

They are written log files they get created once you set file handlers: I don't know what handlers we want, but I imagine some of them will log stuff to console and some to a file. Anyway, feel free to close this request, as I'll be deleting/forking to work on the talent ranking. I'll rewrite logging sometime in the future.

Aldriana commented 13 years ago

Seems to me that checking in log files is probably a bad plan. We should probably add *.log to .gitignore and let the code create them when and if it needs them.