Opened 16 years ago

Closed 16 years ago

Last modified 12 years ago

#5943 closed (fixed)

django-admin.py should work like manage.py if --setting option is provided

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

Description

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 django-admin.py command --settings=blah work just like manage.py invoked on the same project.

Attachments (4)

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

Download all attachments as: .zip

Change History (30)

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

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 "./runtests.py", line 176, in ?
    django_tests(int(options.verbosity), options.interactive, args)
  File "./runtests.py", 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/simple.py", line 142, in run_tests
    create_test_db(verbosity, autoclobber=not interactive)
  File "/opt/local/lib/python2.4/site-packages/django/test/utils.py", line 155, in create_test_db
    call_command('syncdb', verbosity=verbosity, interactive=False)
  File "/opt/local/lib/python2.4/site-packages/django/core/management/__init__.py", line 124, in call_command
    app_name = get_commands()[name]
  File "/opt/local/lib/python2.4/site-packages/django/core/management/__init__.py", line 87, in get_commands
    project_directory = setup_environ(__import__(settings.SETTINGS_MODULE))
  File "/opt/local/lib/python2.4/site-packages/django/core/management/__init__.py", line 254, in setup_environ
    project_module = __import__(project_name, {}, {}, [''])
ImportError: No module named tests

by Todd O'Bryan, 16 years ago

Attachment: django-admin-settings.patch added

comment:2 by Todd O'Bryan, 16 years ago

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 by Russell Keith-Magee, 16 years ago

Resolution: fixed
Status: newclosed

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

comment:4 by Todd O'Bryan, 16 years ago

Has patch: unset
Resolution: fixed
Status: closedreopened

The patch was undone because it kills django-admin.py 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 by jkocherhans, 16 years ago

Has patch: set

This was rolled back by adrian in [6871] because it was breaking django-admin.py 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. django-admin.py is picking up custom commands and django-admin.py 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?

by jkocherhans, 16 years ago

by jkocherhans, 16 years ago

Attachment: django-admin-commands.diff added

comment:6 by Karen Tracey <kmtracey@…>, 16 years ago

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

in reply to:  5 comment:7 by Ben Spaulding, 16 years ago

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 by graham.carlyle@…, 16 years ago

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 by anonymous, 16 years ago

I have applied the attached patch to django-newformsadmin r7818 (manually, it appears the patch chokes on the docstring).

django-admin.py runserver will choke on django apps configured with setuptools namespace (eg: __import__('pkg_resources').declare_namespace(__name__); but django-admin.py runserver --settings=$DJANGO_SETTINGS_MODULE works fine.

comment:10 by Malcolm Tredinnick, 16 years ago

Owner: changed from nobody to Malcolm Tredinnick
Status: reopenednew

comment:11 by Russell Keith-Magee, 16 years ago

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 by Malcolm Tredinnick, 16 years ago

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 by Malcolm Tredinnick, 16 years ago

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 by Russell Keith-Magee, 16 years ago

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

comment:15 by Russell Keith-Magee, 16 years ago

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

comment:16 by Malcolm Tredinnick, 16 years ago

Owner: Malcolm Tredinnick removed

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 by David Cramer, 16 years ago

Cc: dcramer@… added

comment:18 by Michael Radziej, 16 years ago

milestone: post-1.0

comment:19 by Russell Keith-Magee, 16 years ago

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 by Malcolm Tredinnick, 16 years ago

@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 by Russell Keith-Magee, 16 years ago

@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 by Russell Keith-Magee, 16 years ago

Resolution: fixed
Status: newclosed

(In [8282]) Fixed #5943 -- Modified django-admin to behave like manage.py 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 by Ivan Illarionov, 16 years ago

Resolution: fixed
Status: closedreopened

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

by Ivan Illarionov, 16 years ago

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

comment:24 by Ivan Illarionov, 16 years ago

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 by Jacob, 16 years ago

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 by Jacob, 12 years ago

milestone: 1.0 beta

Milestone 1.0 beta deleted

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