Code

Opened 15 months ago

Closed 9 months ago

#19737 closed Cleanup/optimization (wontfix)

Deprecate and then remove "shell" management command

Reported by: carljm Owned by: nobody
Component: Core (Management commands) Version: 1.4
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by carljm)

The only benefit of the "shell" management command is that it saves you from having to set the DJANGO_SETTINGS_MODULE env var, and instead automatically use the one set in manage.py. This is a pretty minor benefit; it's not hard to use your favorite technique (alias, script, whatever) to reduce the number of characters you need to type to run DJANGO_SETTINGS_MODULE=someproj.settings python, with the added advantage that you can easily use any python REPL you like without having to patch "shell" to explicitly support it.

The downside of having "shell" in Django is that it's a non-trivial maintenance burden to decide which REPLs to support, add support for them, and then update/fix that support through the years. We've already seen a steady stream of tickets related to various edge-cases in IPython startup, not to mention the major API changes in recent IPython versions. There's no reason for Django to have to be maintaining code related to IPython.

Attachments (0)

Change History (10)

comment:1 Changed 15 months ago by carljm

  • Description modified (diff)

comment:2 Changed 15 months ago by Alex

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 15 months ago by jezdez

<3

comment:4 follow-up: Changed 11 months ago by jklaiho

While I'm not opposed to this change as such, the statement

"The only benefit of the "shell" management command is that it saves you from having to set the DJANGO_SETTINGS_MODULE env var,"

...isn't entirely correct? After all, manage.py does sys.path insertion as well. I just ran into this today. I've got my apps in my virtualenv's site-packages dir, but my project lives outside of any importable path. I rely on manage.py's insertion of the project dir into sys.path for runserver and shell. Or am I missing something obvious here?

comment:5 in reply to: ↑ 4 Changed 11 months ago by carljm

Replying to jklaiho:

While I'm not opposed to this change as such, the statement

"The only benefit of the "shell" management command is that it saves you from having to set the DJANGO_SETTINGS_MODULE env var,"

...isn't entirely correct? After all, manage.py does sys.path insertion as well. I just ran into this today. I've got my apps in my virtualenv's site-packages dir, but my project lives outside of any importable path. I rely on manage.py's insertion of the project dir into sys.path for runserver and shell. Or am I missing something obvious here?

The stock manage.py since Django 1.4 doesn't touch sys.path at all. Python automatically prepends sys.path with the directory of the currently executing script, so the directory containing manage.py is added to sys.path by Python. But if you just run python directly, Python adds the current directory to sys.path. So in terms of sys.path, running python from the same directory as manage.py has the same effect as running manage.py shell.

comment:6 Changed 11 months ago by jklaiho

Ah, indeed, the sys.path munging I saw in Django code was in the deprecated setup_environ method; running ack sys.path inside django/core/management, I didn't see the deprecation warning there. My mistake.

Nonetheless, the main practical benefit of manage.py here is the ability to alias it into some command that can be run from outside the project directory. I usually have something like this in my virtualenvwrapper postactivate script:

export PROJECT_DIR=/path/to/projectrootandmanagepy
alias ms='python $PROJECT_DIR/manage.py shell'

The upshot of this is that I can just do workon someproject and from then on just invoke ms regardless of which path I happen to be in. I would imagine that this is not an uncommon pattern.

I was about to say how setting DJANGO_SETTINGS_MODULE before running python is not precisely equivalent to manage.py shell because of this, but then I realized an alternative that essentially works the same way without using manage.py:

alias p='cd $PROJECT_DIR; DJANGO_SETTINGS_MODULE=someproject.settings python; cd -'

It's uglier, but after a quick test it seems to work equivalently, returning to the original directory after the Python shell terminates (Windows users will have to improvise). The solution seems obvious now that I thought of it, but it probably isn't immediately obvious to everyone. Hopefully this comment will help someone if they come googling in a similar situation once the shell command is gone.

Version 2, edited 11 months ago by jklaiho (previous) (next) (diff)

comment:8 Changed 9 months ago by timo

Marc's comment from the pull request:

Personally, I can see the argument for deprecating it, but you are asking basically every django user to change something they probably use every single day, not to mention is exceedingly well documented in tutorials everywhere. IMO, if the maintenance is the problem, drop iPython/bPython support and just leave a simple shell command. This may be worth a discussion on -dev I think. It's not a deprecation to be taken lightly.

I tend to agree as it seems like the changes to our tutorial would only create confusion.

comment:9 Changed 9 months ago by loic84

Strong -1 for me.

This DJANGO_SETTINGS_MODULE is a burden IMO, it's overly verbose, and it's everywhere: in the command line, inside manage.py, inside wsgi.py.

When we ask newcomers to try something in the REPL, on top of adding DJANGO_SETTINGS_MODULE=project.settings to their command line, we'll also have to ask them to run the following snippet to work around our lack of a proper init sequence:

from django.db.models.loading import get_models
get_models()

TBH having manage.py as a single entry point is the only thing that makes me bear with the management commands API.

I'm celebrating most of this is env stuffing is being removed from runtests.py in #19941 and with the --selenium argument, please let's not require it for something as common as running a shell.

Last edited 9 months ago by loic84 (previous) (diff)

comment:10 Changed 9 months ago by timo

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

from carljm on IRC:

I think in an ideal world manage.py shell would not exist, but it is widely used and documented, so people are probably right that pragmatically speaking it's not worth deprecating it at this point. We could just leave [ipython/bpython support] until it breaks and deprecate it then rather than continuing to fix/update it.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.