Code

Opened 2 years ago

Last modified 13 months ago

#18243 new Cleanup/optimization

Management shell should make it easy to force LOGGING_CONFIG=None

Reported by: ncoghlan@… Owned by: nobody
Component: Core (Management commands) Version: 1.3
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The standard behaviour of django means that the management interface is largely useless when run on a server where only the web server user has write access to the log files:
http://stackoverflow.com/questions/3980889/django-prevent-logging-permission-errors-from-shell/

The recommended solution is currently to:

  1. Create a separate management_settings.py file that is merely:
       from .settings import *
       LOGGING_CONFIG = None
    
  2. Set up the management command to use management_settings.py instead of the same settings file as the web service (or pass --settings explicitly on the command line)

While that's powerful and effective, an explicit "--nolog" option would be a lot simpler. Also, "--help" should imply "--nolog" to prevent behaviour like the following:

$ python -m pulpdist.manage_site --help
Usage: manage_site.py subcommand [options] [args]                                                                                                           
                                                                                                                                                            
Options:                                                                                                                                                    
  -v VERBOSITY, --verbosity=VERBOSITY                                                                                                                       
                        Verbosity level; 0=minimal output, 1=normal output,                                                                                 
                        2=all output                                                                                                                        
  --settings=SETTINGS   The Python path to a settings module, e.g.                                                                                          
                        "myproject.settings.main". If this isn't provided, the                                                                              
                        DJANGO_SETTINGS_MODULE environment variable will be
                        used.
  --pythonpath=PYTHONPATH
                        A directory to add to the Python path, e.g.
                        "/home/djangoprojects/myproject".
  --traceback           Print traceback on exception
  --version             show program's version number and exit
  -h, --help            show this help message and exit
Traceback (most recent call last):
  File "/usr/lib64/python2.6/runpy.py", line 122, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/usr/lib64/python2.6/runpy.py", line 34, in _run_code
    exec code in run_globals
  File "/usr/lib/python2.6/site-packages/pulpdist/manage_site.py", line 32, in <module>
    execute_from_command_line(sys.argv)
  File "/usr/lib/python2.6/site-packages/django/core/management/__init__.py", line 429, in execute_from_command_line
    utility.execute()
  File "/usr/lib/python2.6/site-packages/django/core/management/__init__.py", line 377, in execute
    sys.stderr.write(self.main_help_text() + '\n')
  File "/usr/lib/python2.6/site-packages/django/core/management/__init__.py", line 239, in main_help_text
    commands = get_commands().keys()
  File "/usr/lib/python2.6/site-packages/django/core/management/__init__.py", line 101, in get_commands
    apps = settings.INSTALLED_APPS
  File "/usr/lib/python2.6/site-packages/django/utils/functional.py", line 276, in __getattr__
    self._setup()
  File "/usr/lib/python2.6/site-packages/django/conf/__init__.py", line 42, in _setup
    self._wrapped = Settings(settings_module)
  File "/usr/lib/python2.6/site-packages/django/conf/__init__.py", line 139, in __init__
    logging_config_func(self.LOGGING)
  File "/usr/lib/python2.6/site-packages/django/utils/dictconfig.py", line 553, in dictConfig
    dictConfigClass(config).configure()
  File "/usr/lib/python2.6/site-packages/django/utils/dictconfig.py", line 352, in configure
    '%r: %s' % (name, e))
ValueError: Unable to configure handler 'debug_log': [Errno 13] Permission denied: '/var/log/pulpdist/debug.log'

Attachments (0)

Change History (8)

comment:1 Changed 2 years ago by claudep

  • Component changed from Uncategorized to Core (Management commands)
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

I'm not convinced that adding a new command line parameter is the right fix.

What about creating a custom logging Handler in your application which would output to file if permissions are sufficient, else write to sys.stderr (or stdout).

import logging

class FailsafeFileHandler(logging.FileHandler):
    def _open(self):
       try:
           return super(CustomFileHandler, self)._open()
       except IOError:
           return None # Should default to StreamHandler sys.stderr

Completely untested, but you get the idea. Then use this handler in your logging config.

comment:2 Changed 2 years ago by ncoghlan@…

Sure, that's another workaround that fixes it *for me* (FWIW, I actually plan to deploy the standard workaround for my own application and use a separate settings module to switch off the logging configuration entirely for the management API). However, I'm definitely not the only one encountering this problem and for someone not already fairly familiar with the logging system, the errors you see in this case are completely baffling. The default behaviour even breaks the output for *--help* if there's a problem with creating a logging handler (as shown in my original message).

The problem is really that activating the logging should be limited to commands where it's appropriate (e.g. runserver, test) and similarly "active" entry points, but that's not what happens: enabling the logging is entirely implicit in importing django.conf, a behaviour *explicitly* recommended against in the logging documentation (http://docs.python.org/howto/logging.html#configuring-logging-for-a-library).

"But Django's a framework, not a library" is no defence: merely *importing* something should not have side effects of that magnitude, it should require an active call.

comment:3 Changed 2 years ago by claudep

The default Django LOGGING setting contains only basic configuration that we want to be configured in any situation (no FileHandler, for example).

If then you begin to configure things differently, you should know what you're doing and code it accordingly (or set proper permissions on the file system).

I'd still appreciate the opinion of another core committer, though.

comment:4 Changed 2 years ago by vsajip

I'm not a core Django committer, but as the maintainer of logging I do agree with Nick that doing logging configuration as a side-effect of import is not good. It's fine to provide a convenience API (ideally, an idempotent one) to configure logging, and to document that; that API can be called either by client code or by Django code in code paths where Django is being run as a management script. The problem with assuming you'll always want to configure logging automagically is that you can't be sure how people want to use Django, which might not be in a conventional way. For example, I've seen people trying to use just the ORM part. And since Django is used as part of a bigger system, that system's developers might want to choose exactly when to configure logging. It should always be under the control of the application (as opposed to library or framework) developer.

comment:5 Changed 2 years ago by anonymous

Thanks Vinay, your input is much appreciated.

I'm still not sure about what you recommend to do.

Currently, the logging configuration is an opt-out process in all cases. As described in the ticket description, an opt-out method can be to set LOGGING_CONFIG to None in the settings file.

Do you think that this should be an opt-in process? Currently, my opinion is that logging configuration is wanted in 95% of use cases, and for the remaining 5%, the current opt-out method is available. I'm afraid that if we don't configure logging by default, this will be forgotten most of the time.

comment:6 Changed 2 years ago by ncoghlan@…

No, I don't think it needs to be opt-in as such, I'd just like to see the *location* of the implicit configuration changed so it doesn't happen as a side effect of importing django.conf.

For example, on the web side, ensuring that logging is configured could be handled by django.core.handlers.wsgi.WSGIHandler()

On the management CLI side, it could be handled by django.core.management.execute_from_command_line() (and done in such a way that the "--help" code path *avoids* initialising logging)

Those are the active entry points I use, but there may be others.

There are a couple of advantages to doing things this way:

  1. It means that a broken logging configuration isn't quite as fatal an error. At the moment, logging config problems mean that *importing Django* fails. By moving the initialisation, the failure instead moves to code paths that are actually likely to *want* the logging enabled, while paths that don't care about logging (like the --help output) won't be affected.
  1. It's easier to control. Instead of having to set global state to affect whether or not the logging gets configured, you can start adding control parameters to the active interfaces (such as an "init_logging" callable parameter for WSGIHandler and execute_from_command_line that can be set to None to disable logging, left at the default to use the logging config from settings.py, or customised to use something else entirely)

comment:7 Changed 2 years ago by claudep

  • Component changed from Core (Management commands) to Core (Other)
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Cleanup/optimization

OK, so let's see what it will look like in a patch :-)

comment:8 Changed 13 months ago by aaugustin

  • Component changed from Core (Other) to Core (Management commands)

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as new
The owner will be changed from nobody to anonymous. Next status will be 'assigned'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.