Opened 2 years ago
Closed 2 years ago
#34778 closed Cleanup/optimization (fixed)
startproject could use find_spec() rather than import_module() to check for conflicts
| Reported by: | Jacob Walls | Owned by: | Jacob Walls |
|---|---|---|---|
| Component: | Core (Management commands) | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
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 (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.
Change History (10)
comment:1 by , 2 years ago
| Resolution: | → needsinfo |
|---|---|
| Status: | new → closed |
comment:2 by , 2 years ago
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.
comment:3 by , 2 years ago
| Resolution: | needsinfo |
|---|---|
| Status: | closed → new |
comment:4 by , 2 years ago
| Resolution: | → wontfix |
|---|---|
| Status: | new → closed |
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!
comment:5 by , 2 years ago
| Resolution: | wontfix |
|---|---|
| Status: | closed → new |
Suggesting another look after forum discussion.
comment:6 by , 2 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:9 by , 2 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
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
startprojectis 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 runstartprojectless than 50 times since 2005). Do you have a use case where the performance ofstartprojectis causing issues?Cheers, Natalia.