Opened 8 years ago

Closed 7 years ago

Last modified 4 years ago

#5943 closed (fixed)

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

Reported by: toddobryan 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:

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 toddobryan 8 years ago.
django-admin-settings-7229.diff (4.4 KB) - added by jkocherhans 7 years ago.
django-admin-commands.diff (3.9 KB) - added by jkocherhans 7 years ago.
5943-django-admin-8295.diff (974 bytes) - added by i_i 7 years ago.

Download all attachments as: .zip

Change History (30)

comment:1 Changed 8 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

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

Changed 8 years ago by toddobryan

comment:2 Changed 8 years ago by toddobryan

  • 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 8 years ago by russellm

  • Resolution set to fixed
  • Status changed from new to closed

(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 Changed 8 years ago by toddobryan

  • Has patch unset
  • Resolution fixed deleted
  • Status changed from closed to reopened

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 follow-up: Changed 7 years ago by jkocherhans

  • 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?

Changed 7 years ago by jkocherhans

Changed 7 years ago by jkocherhans

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

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

comment:7 in reply to: ↑ 5 Changed 7 years ago by benspaulding

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

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

  • Owner changed from nobody to mtredinnick
  • Status changed from reopened to new

comment:11 Changed 7 years ago by russellm

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 7 years ago by mtredinnick

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 7 years ago by mtredinnick

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 7 years ago by russellm

(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 Changed 7 years ago by russellm

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

comment:16 Changed 7 years ago by mtredinnick

  • Owner mtredinnick 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 7 years ago by dcramer

  • Cc dcramer@… added

comment:18 Changed 7 years ago by mir

  • milestone set to post-1.0

comment:19 Changed 7 years ago by russellm

  • milestone changed from post-1.0 to 1.0 beta

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

comment:20 Changed 7 years ago by mtredinnick

@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 7 years ago by russellm

@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 7 years ago by russellm

  • Resolution set to fixed
  • Status changed from new to closed

(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 Changed 7 years ago by i_i

  • Resolution fixed deleted
  • Status changed from closed to reopened

[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.

Changed 7 years ago by i_i

comment:24 Changed 7 years ago by i_i

  • 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 7 years ago by jacob

  • Resolution set to fixed
  • Status changed from reopened to closed

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

  • milestone 1.0 beta deleted

Milestone 1.0 beta deleted

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