Opened 6 years ago

Closed 4 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: master
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 6 years ago.
0001-move-logging-setup-to-after-settings-_really_-is-ful.patch (2.2 KB) - added by Simon Percivall 5 years ago.
fix circular imports.
14861-3.diff (5.3 KB) - added by Claude Paroz 4 years ago.
Move logging config outside of Settings.init
14861-4.diff (5.5 KB) - added by Claude Paroz 4 years ago.

Download all attachments as: .zip

Change History (21)

comment:1 Changed 6 years ago by Russell Keith-Magee

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.

Changed 6 years ago by Adam Vandenberg

Attachment: logging-imports.diff added

comment:2 Changed 6 years ago by Jonas Obrist

Has patch: set

comment:3 Changed 6 years ago by Gabriel Hurley

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 Changed 5 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

Changed 5 years ago by Simon Percivall

fix circular imports.

comment:5 Changed 5 years ago by Simon Percivall

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 Changed 5 years ago by Claude Paroz

Component: DocumentationCore (Other)

comment:7 Changed 4 years ago by Simon Percivall

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 Changed 4 years ago by Claude Paroz

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 Changed 4 years ago by Simon Percivall

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 Changed 4 years ago by Claude Paroz

Owner: nobody deleted
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 Changed 4 years ago by Simon Percivall

Cc: percivall@… added

comment:12 Changed 4 years ago by Claude Paroz

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

Changed 4 years ago by Claude Paroz

Attachment: 14861-3.diff added

Move logging config outside of Settings.init

comment:13 Changed 4 years ago by james@…

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

Changed 4 years ago by Claude Paroz

Attachment: 14861-4.diff added

comment:14 Changed 4 years ago by Claude Paroz

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 Changed 4 years ago by anonymous

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 Changed 4 years ago by Claude Paroz

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 Changed 4 years ago by Claude Paroz <claude@…>

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