#29820 closed Cleanup/optimization (wontfix)
Remove unused, undocumented, and unnecessary dummy template backend
Reported by: | Jon Dufresne | Owned by: | nobody |
---|---|---|---|
Component: | Template system | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The dummy backend is undocumented.
The dummy backend is unused except for testing multiple backend scenarios. For those testing purposes, can use the Jinja2 template engine as a 2nd engine.
Further, the dummy backend isn't providing any other purpose such that the Django or Jinja2 engine wouldn't work as a replacement.
Change History (9)
comment:1 by , 6 years ago
Has patch: | set |
---|
comment:2 by , 6 years ago
It was added as part of Multiple Template Engines project. I guess you would need to propose revising the DEP before making a proposal here.
comment:4 by , 6 years ago
We have dummy backends for all the other swappable backends such as cache, db, and mail. But I guess since this was only in the DEP, and there don't seem to be many alternative template backends being made, it's not providing much value and removing it is simpler.
comment:5 by , 6 years ago
I added the dummy backend for two reasons:
- ensuring consistency with other pluggable backends, as explained by Adam; if we're going to remove it, we should also remove other dummy backends;
- providing a good starting point for people wanting to understand how template backends work; of course it's hard to say if it actually helped contributors.
In addition, the dummy backend made it possible to test multiple template engines without external dependencies — when we were still trying to keep a good chunk of Django functionality without dependencies. I think that argument has weakened since then.
I'm not seeing a lot of value in removing this bit of code, but if there's a strong desire to ship 42 fewer low-maintenance SLoCs, why not, I'm not married to this code :-)
comment:6 by , 6 years ago
ensuring consistency with other pluggable backends, as explained by Adam; if we're going to remove it, we should also remove other dummy backends;
These other dummy backends are currently either documented or used.
The dummy cache backend is documented at:
./docs/ref/settings.txt:151:* ``'django.core.cache.backends.dummy.DummyCache'`` ./docs/topics/cache.txt:336:Dummy caching (for development)
On these pages the backend is recommended for use in development/testing. Personally, I'm a bit unconvinced that using a different cache backend for development/testing is the best idea -- for the same reasons that using a different database backend is bad idea -- as subtle implementation differences won't be noticed until deployed to production. I guess that is besides the point. However, I think this development/testing use case could easily be replaced by the locmem backend in the documentation. Therefore, if its removal is also desired, I think it could be safely deprecated and removed in time.
The dummy email backend is documented at:
./docs/topics/email.txt:552:Dummy backend
Upon inspection, its purpose seems mostly -- if not entirely -- a duplicate of the locmem backend. Therefore, if its removal is also desired, I think it could be safely deprecated and removed in time.
The dummy database backend is used at:
./django/db/utils.py:151: 'ENGINE': 'django.db.backends.dummy', ./django/db/utils.py:155: self._databases[DEFAULT_DB_ALIAS]['ENGINE'] = 'django.db.backends.dummy' ./django/db/utils.py:173: conn.setdefault('ENGINE', 'django.db.backends.dummy') ./django/db/utils.py:175: conn['ENGINE'] = 'django.db.backends.dummy'
The dummy database backend looks to be used to replace a configuration of no databases. I could investigate what removing this would mean. I would have to dive deeper before knowing if this is viable.
So, after that quick research, I'm open to the idea of deprecating and removing these other dummy backends. WDYT? I can investigate what the concrete code changes will mean first if that'll help make a decision.
- providing a good starting point for people wanting to understand how template backends work; of course it's hard to say if it actually helped contributors.
I think there is some value here, but I think it would ultimately be better served through documentation. The current version is:
https://docs.djangoproject.com/en/dev/topics/templates/#custom-backends
Is this known to be lacking anywhere? If so, I can try to improve it.
In addition, the dummy backend made it possible to test multiple template engines without external dependencies ...
I find this to be the most convincing reason to keep it. If we feel test dependencies should be reduced or kept to a minimum, I understand. On the other hand, is there an expectation that developers will install test/requirements/py3.txt
before running tests? The documentation suggests this is so.
I'm not seeing a lot of value in removing this bit of code ...
Ok, no worries. I wanted to highlight undocumented and unused code (outside of tests). I think there is value in removing unused code and shifting the tests towards the more representative configuration scenarios of mixing Jinja2 with Django templates. If the decision is to keep it, that is fine.
comment:7 by , 6 years ago
I think the dummy backends have some usefulness. It seems to me we need some way to represent "nothing configured" for things like database, email, and cache.
comment:8 by , 6 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
It sounds like there is a preference for keeping the backend. I'll close this and the PR to reflect this.
comment:9 by , 6 years ago
For future reference, I think Jon's analysis was interesting and I could support a well-designed plan to remove all dummy backends (if there are tangible benefits).
Yes, there's an expectations that developers will install test requirements. I've seen most contributors do so easily at sprints. The "run tests without dependencies" argument is obsolete.
PR