Opened 11 years ago
Closed 11 years ago
#20549 closed Cleanup/optimization (wontfix)
Refactor to take advantage of get_app_paths()
Reported by: | Aymeric Augustin | Owned by: | |
---|---|---|---|
Component: | Core (Other) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Someday/Maybe | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
b55624a0263e31be80fbe7bb51e47ccd253e7a5d added a get_app_paths()
method in the app cache to simplify fixture loading.
This method could be reused to simplify other code that discovers files at conventional locations inside apps (templates, translations, etc.)
Change History (6)
comment:1 by , 11 years ago
Has patch: | set |
---|---|
Owner: | changed from | to
Status: | new → assigned |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 11 years ago
I've also looked at the get_javascript_catalog
view as it also traverses app directories, but I'm confused by its behavior. It takes a packages
param and then re-declares it in a strange conditional list comprehension. It also doesn't reverse the packages list, is it's behavior different from trans_real
? I'm not sure if get_app_paths
can work there...
I also thought that contrib.staticfiles
would be a good candidate (the docstring for get_app_paths mentions it), but it doesn't seem feasible without changing the current AppDirectoriesFinder
/AppStaticStorage
API.
django/contrib/staticfiles/finders.py
django/contrib/staticfiles/storage.py
Is changing the first two code paths an acceptable fix for this ticket? Do you know of more places where get_app_paths
should be used?
One last note: get_app_paths
actually returns paths to models.pyc
, that was not what I expected. In loaddata
's fixture_dirs
there's some calls to os.path.abspath
, os.path.realpath
and os.path.dirname
, maybe this should all be done in get_app_paths
itself?
comment:3 by , 11 years ago
Mmmm, running the tests again and AppResolutionOrderI18NTests
still fails, thought I fixed it but apparently not. Not sure what causes the failure though...
comment:4 by , 11 years ago
I didn't work on this ticket because I noticed that most places that could take advantage of get_app_paths
actually depend on the list of applications in INSTALLED_APPS
, and not on the list of applications in the app cache.
And it's currently possible for the app cache to contain applications that aren't in INSTALLED_APPS
. Switching to get_app_paths
could change the results of the lookups by adding more applications to the search list.
Until app loading is in a better shape, I'm not sure we can move on with this.
comment:5 by , 11 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
Triage Stage: | Accepted → Someday/Maybe |
Cool, was thinking this would be an easy one ;) Not going to put more time into this then.
comment:6 by , 11 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
get_app_paths()
was deprecated during the app-loading refactor. This isn't relevant any more.
I've identified two locations that were fairly easy to convert:
AppResolutionOrderI18NTests
so it used@override_settings
)