[Django] #30098: Permit using packages (directories) in custom django-admin commands.
Django
-------------------------------------+-------------------------------------
Reporter: Andrey | Owner: Andrey Volkov
Volkov |
Type: New | Status: assigned
feature |
Component: Core | Version: 2.1
(Management commands) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Current Django behavior is that only files are allowed for custom django-
admin commands definition. It would be great if Django permitted
directories (see example below).
Current (required) style:
{{{
app_example/management/commands
├── first_command.py
└── second_command.py
}}}
New style:
{{{
app_example/management/commands
├── first_command
│ ├── __init__.py
│ └── some_useful_data
└── second_command
└── __init__.py
}}}
Or combined style:
{{{
app_example/management/commands
├── first_command
│ ├── __init__.py
│ └── some_useful_data
└── second_command.py
}}}
This feature allows developers to use additional files (some_useful_data
is this case) in structured way, also providing encapsulation.
The only code to be changed is in file
`django/core/management/__init__.py` line 27:
From `if not is_pkg and not name.startswith('_')]`
To `if not name.startswith('_')]`
After this change, function `find_commands` won't ignore packages
(directories).
Also, backward compatibility will be saved (because single files work in
the same way).
--
Ticket URL: <https://code.djangoproject.com/ticket/30098>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
| Volkov
Type: New feature | Status: assigned
Component: Core (Management | Version: 2.1
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Andrey Volkov):
Now I am working on tests and documentation.
--
Ticket URL: <https://code.djangoproject.com/ticket/30098#comment:1>
Django
-------------------------------------+-------------------------------------
| Volkov
Type: New feature | Status: assigned
Component: Core (Management | Version: 2.1
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
At first glance, I'm not in favor of the extra complexity this would
introduce. Django stores helper files for its own commands in the
`management` directory (`django/core/managment`) which seems fine.
--
Ticket URL: <https://code.djangoproject.com/ticket/30098#comment:2>
Django
-------------------------------------+-------------------------------------
| Volkov
Type: New feature | Status: assigned
Component: Core (Management | Version: 2.1
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Andrey Volkov):
Replying to [comment:2 Tim Graham]:
> At first glance, I'm not in favor of the extra complexity this would
introduce. Django stores helper files for its own commands in the
`management` directory (`django/core/managment`) which seems fine.
Yeah, this can increase complexity, but this feature is not obligatory.
Developers can write management commands the same way as before.
Why I want this change, is because it solves and/or simplifies some
problems (mine too). For example, we could have a library that assumes
some filenames to be `data.json` and we want to use this library in the
number of commands. With directories style it can be solved like this:
{{{
app_example/management/commands
├── first_command
│ ├── data.json
│ └── __init__.py
└── second_command
├── data.json
└── __init__.py
}}}
With files-only style it (with some changes to library behavior) should
be:
{{{
app_example/management
├── commands
│ ├── first_command.py
│ └── second_command.py
├── first_command_data.json
└── second_command_data.json
}}}
Last solution gives duplicated command names. That means if developer
changes a command name, it should be changed at least twice (against one
change in directory approach).
Also, this style gives possibility for encapsulation. Django wouldn't know
how the command is structured (is it file or directory) it will just use
it as a regular module, that contains a `Command` class.
Last point I want to mention here, is that it can help Django in the
future. If additional functionality will be given to the management module
(some features except commands), encapsulation will prevent storing random
files (that refer only to commands) in management directory. And change,
that enables directory-based commands, is extremely small, so that it
affects only parsing the `app/management/commands` directory (it is
literally removes only one condition).
Sorry for such a long comment, but I really think that this feature will
help developers both now and in the future.
--
Ticket URL: <https://code.djangoproject.com/ticket/30098#comment:3>
Django
-------------------------------------+-------------------------------------
| Volkov
Type: New feature | Status: assigned
Component: Core (Management | Version: 2.1
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Andrey Volkov):
Patch is finished. Pull request is submitted.
https://github.com/django/django/pull/10836
--
Ticket URL: <https://code.djangoproject.com/ticket/30098#comment:4>
Django
-------------------------------------+-------------------------------------
| Volkov
Type: New feature | Status: assigned
Component: Core (Management | Version: 2.1
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/30098#comment:5>
Django
-------------------------------------+-------------------------------------
| Volkov
Type: New feature | Status: assigned
Component: Core (Management | Version: 2.1
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
Comment:
Documentation fails its build, because django is considered as
misspelling.
--
Ticket URL: <https://code.djangoproject.com/ticket/30098#comment:6>
Django
-------------------------------------+-------------------------------------
| Volkov
Type: New feature | Status: assigned
Component: Core (Management | Version: 2.1
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
Comment:
Fixed documentation. All checks have passed.
--
Ticket URL: <https://code.djangoproject.com/ticket/30098#comment:7>
Django
-------------------------------------+-------------------------------------
| Volkov
Component: Core (Management | Version: 2.1
commands) |
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => wontfix
Comment:
Hi Andrey. Thanks for the suggestion and PR demonstrating it. As it stands
I think I agree with Tim's initial comment.
I'm not sure I see any real benefit to allowing packages here. It's
already possible to add a package to contain any supporting files (if
there's more than one) — either in `management` or in `commands` — so I'm
not convinced any extra encapsulation merits the change. (I'm not sure I
buy either the ''renaming'' or ''extra features'' points you make.)
The restriction to modules has been there ≈forever. See
f25b8cdbcdc99812299e9081cd3b03ddc08dcf74 for #5222, which introduced
`find_commands()`. Just on the principle that ''There should be one-- and
preferably only one --obvious way to do it.'' I don't think changing that
design choice is merited.
Also, people will be using the fact that you **can** put a package in
`commands` and not have it detected. If we change that such packages will
be picked up by `find_commands()` and we'll have a whole load of reports
of `help` listing phantom commands that only lead to `AttributeError:
module '...' has no attribute 'Command'` errors when invoked.
So for both these reasons I'm going to opt for `wontfix` here. Possibly
you could raise it on the DevelopersMailingList if you feel strongly on
the issue.
Thanks again.
--
Ticket URL: <https://code.djangoproject.com/ticket/30098#comment:8>