chireiden / shanghai

Shanghai
GNU General Public License v3.0
4 stars 2 forks source link

Add initial primitive logging capabilities #22

Closed nitori closed 8 years ago

nitori commented 8 years ago

As title states, adds logging capabilities.

A context and name as strings are passed to the get_logger function to create a logger and a new logging file. It's primarily so we can have multiple logs per network per channel etc.

I'm still unsure on how it's done. To be able to log practically everywhere you'd have to hand around the logger instances or some other information to be able to recreate the logger instances. For example, to get rid of the print lines in the Connection class I had to either create a new logging instance for that connection, which is silly, because a connection is tied to the network and should be handled by the networks logger (so it appears in the correct file) - or pass the logger to the Connection either via __init__ or a separate method (which I did).

A similar problem is in irc/message.py. A Message shouldn't really do the logging itself, because a Message doesn't really know currently whether it's a channel or user and what network it's from. So a possibility would be to raise an exception and let the network handle the logging.

Another way (which I kinda like) would be to do it the Flask (web framework) way and use a global proxy object, that fetches the current logger for the current context.

The way Flask does it, is by using a "LocalProxy" instance that is tied to a function that gets (in this case) the current app from the top of a stack. You would push and pop from that stack whenever you enter or leave a new app context - or in our case, whenever we enter and leave a logging context.

If we do it like that, we would simply import the "current_logger" using something like: from .logging import current_logger or the like. And to create a new logging context we would maybe do something like: from .logging import LogContext create our instance with some arguments and simply: log_context.push() and log_context.pop() on entering and leaving the new logging context. Any statements like current_logger.info('Foobar') between the push and pop would then log using the logger that was created by the LogContext (somewhere in __init__ or the like).

Edit: also, we could do stuff like:

with LogContext('channel', message.target):
    handle_channel_message(message)
nitori commented 8 years ago

So, now I actually added the context based approach, that I explained above. Works so far. But because of the asyncio way we do things, we have to be be careful to always leave the log context at the right places.

FichteFoll commented 8 years ago

Took a brief look and looks good so far, but I'd need to experiment some in order to comment on the full scope, which I don't have time for atm (especially the LocalProxy stuff).

nitori commented 8 years ago

I decided to make it set a default global configuration in main. That way you can create a new logging context, without the need of passing down the current configuration to wherever you create the new context.

Also, I really like the new {**foo_dict, **bar_dict} syntax to merge to dicts :D

nitori commented 8 years ago

Oh just saw that there is an issue for logging: #15