[Django] #34778: startproject could use find_spec() rather than import_module() to check for conflicts
Django
conflicts
-------------------------------------+-------------------------------------
Reporter: Jacob | Owner: nobody
Walls |
Type: | Status: new
Cleanup/optimization |
Component: Core | Version: dev
(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 |
-------------------------------------+-------------------------------------
The `startproject` command currently depends on calling
`importlib.import_module()` on the intended new project name, which will
execute a python module if it happens to find one.
It would be more performant and less intrusive to check the return value
of `importlib.util.find_spec`
[https://docs.python.org/3/library/importlib.html?highlight=find_spec#importlib.util.find_spec
(docs)], which avoids importing anything (so long as the path doesn't
include a dot, but that can be checked for first.)
Happy to submit a PR if welcome.
--
Ticket URL: <https://code.djangoproject.com/ticket/34778>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
conflicts
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization |
Component: Core (Management | Version: dev
commands) |
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => needsinfo
Comment:
Hello Jacob, thank you for your ticket.
As a general rule, in order to decide about changes that would impact
performance, the usual advice is to provide, next to the change, some
evidence that the change will actually improve the performance. To do so,
one option is to use https://github.com/django/djangobench.
For this specific suggestion, and considering that `startproject` is used
once per project setup, I find it hard to justify to change the code on a
script that a django user may run 10? 50? 100? times in their career (I
personally have run `startproject` less than 50 times since 2005). Do you
have a use case where the performance of `startproject` is causing issues?
Cheers, Natalia.
--
Ticket URL: <https://code.djangoproject.com/ticket/34778#comment:1>
Django
conflicts
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Core (Management | Version: dev
commands) |
Severity: Normal | Resolution: needsinfo
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 Jacob Walls):
Hi Natalia, thanks for the response.
I shouldn't have used the word peformant (sorry!) as performance was not
the real motivation. The motivation was that it's simply less invasive to
avoid executing unnecessary modules. If you develop a project called
`presto` and decide to create a project `presto-helper` without the
knowledge that there's someone else's `presto-helper` already installed in
your environment, `presto-helper` will get executed, which could possibly
dump things to the terminal or execute queries (hopefully it wouldn't). It
just seems nicer to avoid that if there's a simple workaround.
I understand this is a judgment call since users are responsible for
what's installed in their environment, and if they install things that
have noisy import-time statements, that's not on Django.
--
Ticket URL: <https://code.djangoproject.com/ticket/34778#comment:2>
Django
conflicts
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: nobody
Cleanup/optimization |
Component: Core (Management | Version: dev
commands) |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* resolution: needsinfo =>
--
Ticket URL: <https://code.djangoproject.com/ticket/34778#comment:3>
Django
conflicts
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: nobody
Cleanup/optimization |
Component: Core (Management | Version: dev
commands) |
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => wontfix
Comment:
Right, I agree with your final sentence, in that Django can't account or
solve all corner cases. It provides solutions for the general and
(common?) case, and then for these situations, we usually prefer to keep
the code simple and straightforward.
Having said that, and after reading the docs, it might be a decent idea to
switch, though I fear I may be missing potential side effects of this
change. Would you consider posting to the Django Internals forum category
so more people can chime in? If no one objects in a week or two, we could
accept the ticket.
I'll close as wontfix for now to clean up the pending triage queue, but do
ping back in a couple of weeks so we revisit this proposal.
Thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/34778#comment:4>
Django
conflicts
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: nobody
Cleanup/optimization |
Component: Core (Management | Version: dev
commands) |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* resolution: wontfix =>
Comment:
Suggesting another look after [https://forum.djangoproject.com/t/check-
for-name-clashes-in-startproject-without-importing-modules/23091 forum
discussion].
--
Ticket URL: <https://code.djangoproject.com/ticket/34778#comment:5>
Django
conflicts
-------------------------------------+-------------------------------------
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Core (Management | Version: dev
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
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):
* owner: nobody => Jacob Walls
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/34778#comment:6>
Django
conflicts
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Core (Management | Version: dev
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/17216 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/34778#comment:7>
Django
conflicts
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Core (Management | Version: dev
commands) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
Comment:
Tentatively accepted.
--
Ticket URL: <https://code.djangoproject.com/ticket/34778#comment:8>
Django
conflicts
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Core (Management | Version: dev
commands) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/34778#comment:9>
Django
conflicts
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
Type: | Walls
Component: Core (Management | Version: dev
commands) |
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"bcd80de8b5264d8c853bbd38bfeb02279a9b3799" bcd80de]:
{{{
#!CommitTicketReference repository=""
revision="bcd80de8b5264d8c853bbd38bfeb02279a9b3799"
Fixed #34778 -- Avoided importing modules in startapp/startproject.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34778#comment:10>