Opened 9 years ago

Closed 8 years ago

Last modified 5 years ago

#5943 closed (fixed) should work like if --setting option is provided

Reported by: Todd O'Bryan Owned by:
Component: Core (Management commands) Version: master
Severity: Keywords:
Cc: dcramer@… Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:


Right now, app-provided commands aren't included, startproject is still available, and startapp uses the current directory rather than the project directory.

The attached patch makes command --settings=blah work just like invoked on the same project.

Attachments (4)

django-admin-settings.patch (4.5 KB) - added by Todd O'Bryan 9 years ago.
django-admin-settings-7229.diff (4.4 KB) - added by jkocherhans 9 years ago.
django-admin-commands.diff (3.9 KB) - added by jkocherhans 9 years ago.
5943-django-admin-8295.diff (974 bytes) - added by Ivan Illarionov 8 years ago.

Download all attachments as: .zip

Change History (30)

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

Needs documentation: unset
Needs tests: unset
Patch needs improvement: set
Triage Stage: UnreviewedAccepted

Thanks, Todd. The patch appears to break the Django system tests; when I run the system tests, I get the following stack trace:

Traceback (most recent call last):
  File "./", line 176, in ?
    django_tests(int(options.verbosity), options.interactive, args)
  File "./", line 147, in django_tests
    failures = run_tests(test_labels, verbosity=verbosity, interactive=interactive, extra_tests=extra_tests)
  File "/opt/local/lib/python2.4/site-packages/django/test/", line 142, in run_tests
    create_test_db(verbosity, autoclobber=not interactive)
  File "/opt/local/lib/python2.4/site-packages/django/test/", line 155, in create_test_db
    call_command('syncdb', verbosity=verbosity, interactive=False)
  File "/opt/local/lib/python2.4/site-packages/django/core/management/", line 124, in call_command
    app_name = get_commands()[name]
  File "/opt/local/lib/python2.4/site-packages/django/core/management/", line 87, in get_commands
    project_directory = setup_environ(__import__(settings.SETTINGS_MODULE))
  File "/opt/local/lib/python2.4/site-packages/django/core/management/", line 254, in setup_environ
    project_module = __import__(project_name, {}, {}, [''])
ImportError: No module named tests

Changed 9 years ago by Todd O'Bryan

Attachment: django-admin-settings.patch added

comment:2 Changed 9 years ago by Todd O'Bryan

Patch needs improvement: unset

I think the new patch works. (At least, it passes the tests.) I'd missed an ImportError and forgot that it might be possible to have INSTALLED_APPS, but not a project_directory (as in the tests).

Please check lines 87 and 92 carefully. I think those are the only errors that could come up during the try blocks, but I might have missed something.

comment:3 Changed 9 years ago by Russell Keith-Magee

Resolution: fixed
Status: newclosed

(In [6718]) Fixed #5943 -- Modified to work like whenever a --settings option is provided. Thanksfor the patch, Todd O'Bryan.

comment:4 Changed 9 years ago by Todd O'Bryan

Has patch: unset
Resolution: fixed
Status: closedreopened

The patch was undone because it kills when the --settings option isn't provided, for example, if it's given using the DJANGO_SETTINGS_MODULE environment variable. I now have time again to look at what the problem is.

comment:5 Changed 9 years ago by jkocherhans

Has patch: set

This was rolled back by adrian in [6871] because it was breaking runserver. The current patch wouldn't apply cleanly to [7229], but I went through and applied it by hand and it seems to work fine for me. is picking up custom commands and runserver works fine both with a --settings flag and the DJANGO_SETTINGS_MODULE environment variable. I've attached a new version of the patch that applies to [7229]. I'm tempted to just check this in, but I feel like maybe I'm missing something. Anyone want to apply this and try to get it to break?

Changed 9 years ago by jkocherhans

Changed 9 years ago by jkocherhans

Attachment: django-admin-commands.diff added

comment:6 Changed 8 years ago by Karen Tracey <kmtracey@…>

Marked #7518 ( can't createsuperuser) a dup of this since it sound like this change should fix it.

comment:7 in reply to:  5 Changed 8 years ago by Ben Spaulding

Replying to jkocherhans:

Anyone want to apply this and try to get it to break?

I have applied the patch. It applied cleanly and, though I didn’t test every command, runserver and syncdb (creating a new database and first superuser) work with both DJANGO_ADMIN_MODULE and the --settings option.

comment:8 Changed 8 years ago by graham.carlyle@…

I have applied the patch cleanly and has been working fine for me so far. Works ok with some custom commands I have as well.

comment:9 Changed 8 years ago by anonymous

I have applied the attached patch to django-newformsadmin r7818 (manually, it appears the patch chokes on the docstring). runserver will choke on django apps configured with setuptools namespace (eg: __import__('pkg_resources').declare_namespace(__name__); but runserver --settings=$DJANGO_SETTINGS_MODULE works fine.

comment:10 Changed 8 years ago by Malcolm Tredinnick

Owner: changed from nobody to Malcolm Tredinnick
Status: reopenednew

comment:11 Changed 8 years ago by Russell Keith-Magee

FYI, Malcolm - I've been working on this (as well as #6017), but I forgot to claim the ticket. I've been trying to write a unit test suite that will let us test the various ways that we can break django-admin, so that we can avoid the nasty issues that have plagued django-admin in the past.

comment:12 Changed 8 years ago by Malcolm Tredinnick

Regarding the anonymous comment 9: "appears to choke" is not a particularly descriptive failure mode. What goes wrong?

Could you please provide a short description of how to repeat the problem so that we can investigate it. A lot of use don't use eggs or setuptools, so some assistance in helping us repeat the problem will help us to fix it. Thanks.

comment:13 Changed 8 years ago by Malcolm Tredinnick

Reading this patch, I notice it tries to slip in the evil "don't set DJANGO_SETTINGS_MODULE if it's already set" change. This cannot be the right thing to do. It means you can easily end up with the --settings=... value set to one thing and DJANGO_SETTINGS_MODULE set to another thing, which will lead to inconsistent behaviour in subprocesses for example. The two should mirror each other precisely. Ideally, we shouldn't need the environment variable at all, but since we have it, we must keep a consistent environment throughout the process space.

People keep trying to make this change because they want something over there to use their DJANGO_SETTINGS_MODULE when its called, but not right now when they pass in --settings. Those people are setting themselves and everybody else up for difficult-to-diagnose bugs and there's really no logic behind it. Certainly those ideas have their place. But that place is on a desert island. In the middle of the ocean. With no internet connection.

comment:14 Changed 8 years ago by Russell Keith-Magee

(In [7876]) Refs #5943, #6107 -- Added framework and tests to check for the correct operation of django-admin and Thanks for Simon Willison and Karen Tracey for their contributions and assistance testing this patch.

comment:15 Changed 8 years ago by Russell Keith-Magee

Bah - that last checkin comment was dyslexic - it should have read #6017.

comment:16 Changed 8 years ago by Malcolm Tredinnick

Owner: Malcolm Tredinnick deleted

I'm not going to have time to work on this in the near future, so reassigning back to the pool in case anybody wants to work out the egg issue. At some point the solution might be to commit it to force people to see the problem if they've chosen to use eggs, but that will require somebody with time to field the bug reports.

comment:17 Changed 8 years ago by David Cramer

Cc: dcramer@… added

comment:18 Changed 8 years ago by Michael Radziej

milestone: post-1.0

comment:19 Changed 8 years ago by Russell Keith-Magee

milestone: post-1.01.0 beta

This one is actually on my radar; it's the cause of a few nasty problems with external commands.

comment:20 Changed 8 years ago by Malcolm Tredinnick

@russellm: I think I'm fairly happy with this change. I'd be prepared to commit it now and if there really are problems with the eggs case, people will then report them because it will bite them (i.e. a scorched earth policy here). You have any real issues with landing this?

comment:21 Changed 8 years ago by Russell Keith-Magee

@malcolmt: Scorched earth sounds like the right approach for the eggs problem.

As for the rest of the patch - it turns out that this was unrelated to the external commands problem I was chasing. I agree with your previous comment that the mangling of DJANGO_SETTINGS_MODULE should be left out of this change. Other than that, the patch looks good to me.

This leaves 15 test failures in the admin_scripts suite. I haven't dug into them in depth to be certain, but on the surface it looks like relatively simple expected changes to behaviour (i.e., tests previously asserted that django-admin couldn't run user commands, but now it can). I'll proabably get a chance to look at this tomorrow, but if you beat me to it, I won't hold a grudge :-)

comment:22 Changed 8 years ago by Russell Keith-Magee

Resolution: fixed
Status: newclosed

(In [8282]) Fixed #5943 -- Modified django-admin to behave like if settings are provided, either as --settings or DJANGO_SETTINGS_MODULE. Thanks to Joseph Kocherhans and Todd O'Bryan for their work on this ticket.

comment:23 Changed 8 years ago by Ivan Illarionov

Resolution: fixed
Status: closedreopened

[8282] breaks startapp if called from outside project directory, e.g. myproject/ startapp won't work. I wrote the patch to fix this and other issues. Will attach after testing.

Changed 8 years ago by Ivan Illarionov

Attachment: 5943-django-admin-8295.diff added

comment:24 Changed 8 years ago by Ivan Illarionov

Needs documentation: set

Also I think we need to document that users have to unset DJANGO_SETTINGS_MODULE environment variable in order to use startproject command and use startapp in the current directory. English is not my native language, so it would be nice if someone else will do it.

comment:25 Changed 8 years ago by Jacob

Resolution: fixed
Status: reopenedclosed

i_i, please open a new ticket to track this. In general, it's bad form to reopen a ticket closed by a core dev, and doubly-so to attach new patches -- it makes tracking history very hard.

comment:26 Changed 5 years ago by Jacob

milestone: 1.0 beta

Milestone 1.0 beta deleted

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