Opened 13 years ago

Closed 11 years ago

#14861 closed Bug (fixed)

Importing settings in a module that contains a logging Handler causes circular import.

Reported by: donspaulding Owned by: Claude Paroz <claude@…>
Component: Core (Other) Version: dev
Severity: Normal Keywords:
Cc: percivall@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

If settings.py contains the following config for LOGGING:

LOGGING = {
  'version': 1,
  'handlers': {
    'custom_handler': {
      'level': 'INFO',
      'class': 'myproject.logconfig.MyHandler',
    }
  }
}

and myproject/logconfig.py has the following line before the MyHandler definition:

from django.conf import settings

The dictconfig module spits out a none-too-helpful traceback:

Traceback (most recent call last):
  File "manage.py", line 11, in <module>
    execute_manager(settings)
  File "/home/don/myproject/src/django/django/core/management/__init__.py", line 438, in execute_manager
    utility.execute()
  File "/home/don/myproject/src/django/django/core/management/__init__.py", line 379, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/don/myproject/src/django/django/core/management/__init__.py", line 252, in fetch_command
    app_name = get_commands()[subcommand]
  File "/home/don/myproject/src/django/django/core/management/__init__.py", line 101, in get_commands
    apps = settings.INSTALLED_APPS
  File "/home/don/myproject/src/django/django/utils/functional.py", line 276, in __getattr__
    self._setup()
  File "/home/don/myproject/src/django/django/conf/__init__.py", line 41, in _setup
    self._wrapped = Settings(settings_module)
  File "/home/don/myproject/src/django/django/conf/__init__.py", line 126, in __init__
    logging_config_func(self.LOGGING)
  File "/home/don/myproject/src/django/django/utils/dictconfig.py", line 553, in dictConfig
    dictConfigClass(config).configure()
  File "/home/don/myproject/src/django/django/utils/dictconfig.py", line 352, in configure
    '%r: %s' % (name, e))
ValueError: Unable to configure handler 'custom_handler': Unable to configure handler 'custom_handler': 'module' object has no attribute 'logconfig'

The problem is easily rectified once you realize that it's a circular import. Apart from lazily loading the logging config, I'm not even sure there's an appropriate code fix. I'm mainly filing this bug to see if a note can be placed alongside the logging configuration docs that warns of the potential problem.

Attachments (4)

logging-imports.diff (1.3 KB ) - added by Adam Vandenberg 13 years ago.
0001-move-logging-setup-to-after-settings-_really_-is-ful.patch (2.2 KB ) - added by Simon Percivall 12 years ago.
fix circular imports.
14861-3.diff (5.3 KB ) - added by Claude Paroz 12 years ago.
Move logging config outside of Settings.init
14861-4.diff (5.5 KB ) - added by Claude Paroz 11 years ago.

Download all attachments as: .zip

Change History (21)

comment:1 by Russell Keith-Magee, 13 years ago

Component: UncategorizedDocumentation
milestone: 1.3
Triage Stage: UnreviewedAccepted

I don't think there's much we can do here beyond having a documentation note warning of the problem.

by Adam Vandenberg, 13 years ago

Attachment: logging-imports.diff added

comment:2 by Jonas Obrist, 13 years ago

Has patch: set

comment:3 by Gabriel Hurley, 13 years ago

Resolution: fixed
Status: newclosed

In [15449]:

Fixed #14861 -- Added an admonition about the potential for circular imports with custom logging handlers (which produces an extremely confusing error message) to the logging docs. Thanks to donspaulding for the report and adamv for the patch.

comment:4 by Jacob, 12 years ago

milestone: 1.3

Milestone 1.3 deleted

by Simon Percivall, 12 years ago

fix circular imports.

comment:5 by Simon Percivall, 12 years ago

Easy pickings: unset
Resolution: fixed
Severity: Normal
Status: closedreopened
Type: Uncategorized
UI/UX: unset

Actually, moving the logging setup from Settings.__init__ to below self._wrapped = Settings(settings_module) in LazySettings._setup fixes the circular import (at least it fixed my problem). See the attached patch.

comment:6 by Claude Paroz, 12 years ago

Component: DocumentationCore (Other)

comment:7 by Simon Percivall, 12 years ago

Ping.

Sorry for the no-content comment, but this is still very much a problem, and it's a simple fix, and it comes with a patch.

comment:8 by Claude Paroz, 12 years ago

Could you check if you can still reproduce with latest development code? And then, a sample project to demonstrate that your patch is solving the issue would be appreciated.

comment:9 by Simon Percivall, 12 years ago

I thought I would get notified by just commenting. Apparently not.

It's still reproducible with 1.5 HEAD.

A sample project? How? A zip? Sure, if it's necessary.

It's pretty easy to see, however, by just looking at the code, that referencing settings from the logging config, directly or by importing a module that does, before _wrapped is set will cause infinite recursion. And since my patch moves logging setup to after _wrapped is set, the issue is resolved.

comment:10 by Claude Paroz, 12 years ago

Owner: nobody removed
Status: reopenednew
Type: UncategorizedBug

OK, no need for any more examples, I've been able to create a test case to reproduce the issue. I will propose a patch soon (including yours).

I thought I would get notified by just commenting. Apparently not.

You need to put yourself in the cc field to get notified.

comment:11 by Simon Percivall, 12 years ago

Cc: percivall@… added

comment:12 by Claude Paroz, 12 years ago

Attached patch containing simonpercivall's patch, test, and removal of [66312066a01af1325280b07d8e6942b03a46d650].

by Claude Paroz, 12 years ago

Attachment: 14861-3.diff added

Move logging config outside of Settings.init

comment:13 by james@…, 12 years ago

What if my log handler wants to use the models? by declaring an import of models will cause circular import of settings.

by Claude Paroz, 11 years ago

Attachment: 14861-4.diff added

comment:14 by Claude Paroz, 11 years ago

I've just updated my patch, where I confined the logging setup in a private method and I also let a potential circular import warning in the documentation.

comment:15 by anonymous, 11 years ago

I don't think there's a risk of circular import here. Settings are fully configured, the logging setup in itself has no dependency on models. Where would the risk come from?

comment:16 by Claude Paroz, 11 years ago

OK, I can commit it without the admonition, then we'll see if any further issue is raised, we can always add it later.

comment:17 by Claude Paroz <claude@…>, 11 years ago

Owner: set to Claude Paroz <claude@…>
Resolution: fixed
Status: newclosed

In fc69fff9ab5bba8a6cff3eaf51f02a3204b1c015:

Fixed #14861 -- Moved logging config outside of Settings.init

Thanks donspaulding for the report and simonpercivall for the
initial patch.

Note: See TracTickets for help on using tickets.
Back to Top