Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enabled per-module logging level settings #41

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

builtinnya
Copy link
Contributor

These changes make debugging modules easier.

@builtinnya
Copy link
Contributor Author

I've also added a bug fix in disposing output of Skype.
This bug creates a file named "1" in sevabot/

@miohtama
Copy link
Member

Nice! I think we can make this little bit simpler.

Python has internal config file format to configure loggers:

http://docs.python.org/2/library/logging.config.html

We just create logging.conf which is read on the start up and "outsource" all logging configuration for it. It has more powerful syntax and people are more familiar with it, because the format is used in other applications too.

In fact we can rip off some existing logging configuration options from settings.py and just put into this file.

This reduces the amount of code we need to maintain ourselves.

Also, there is another trick:

  logger = logging.getLogger(__name__) # Takes logger name automatically from the module name

... useful for modules get renamed and shuffled around.

@builtinnya
Copy link
Contributor Author

Outsourced all logging configurations to logging.conf.

These changes show just a simple way to do it.

Some design decisions and considerations:

  • Official logging HOWTO recommends using dictConfig() for new applications and deployments
    because this method insulates configuration information from its representation, but in our case
    there seems to be little benefit against additional complexity.
  • Users are required to copy logging.conf.example to logging.conf just like settings.py.
    This is a little bit cumbersome, but including logging.conf directly in the repo is not acceptable.
    Is it better to hard-code default logging configuration or to include the default alternative configuration file
    for the case logging.conf is not given?

@miohtama
Copy link
Member

I think we can just ship with one logging.conf and set it to gitingore so users don't push this around.

settings.conf.example is with .example extension only because it contains a default password. Logging configuration does not have similar security risk, so we can happily have default logging.conf in place.

@miohtama
Copy link
Member

Mmm... I just give it a thought regarding settings.py.example.

We could ship with settings.py. On startup, Sevabot reads the file. If there is a default password, it is changed to random string which is then printed to stderr and settings are reloaded. This way we can avoid unnecessary manual password set process and make the bot initial start up process streamlined. Some work though, and not very high priority.

@builtinnya
Copy link
Contributor Author

I think .gitignore ignores only untracked files and making it work with tracked files isn't easy as
https://gist.github.com/canton7/1423106 suggests.

@builtinnya
Copy link
Contributor Author

Anyway we have to gitignore logging.conf :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants