add_to_builtins in settings.py fails #116

Open
vanschelven opened this Issue Nov 2, 2011 · 5 comments

Projects

None yet

3 participants

@vanschelven

Hi,

When using the django-uni-form templatetags through add_to_builtins in settings.py (the most logical location for using add_to_builtins) the server fails to start.

This is a new bug since after 0.7.0.

It is caused by the use of get_template in the templatetags modules. Since get_template is called at module load-time and get_template requires settings.py to have been fully loaded your code cannot be imported from settings.py.

An example of a call to get_template at module root (i.e. not inside some function) is:
https://github.com/pydanny/django-uni-form/blob/master/uni_form/templatetags/uni_form_tags.py#L142
But there are more, grepping will quickly reveal them.

A simple solution would be to make the associated variables lazily initialized, i.e. set them as None at module-load time and always access them via some getter that checks their value. If necessary I could provide this code for you.

@maraujop
Collaborator

Confirmed, you are right.

I have to say I didn't know of the existence of add_to_builtins, I wish I had before. Anyway I will do a good use of it, great tip.

get_template calls were done globally to sped up django-uni-form when not using template cache loader, If I recall correctly that happened in version 0.8.0.

django-uni-form is going to be replaced by django-crispy-forms https://github.com/maraujop/django-crispy-forms Although it hasn't been publicly announced yet.

I'm not updating fixes to django-uni-form anymore. Could you please have a look at it, maybe upgrade to it (instructions), latest version is 1.0.0 (tests are passing) and submit the pull request there. I won't have time to fix this myself for a couple days.

Thanks, cheers
Miguel

@pydanny
Collaborator

@marujop, there is a reason why people don't use add_to_builtins more. It is a maintenance nightmare in the making. It is one of the warning signs when you review projects, if they have it, then odds are they are being 'clever' in other places.

"Explicit is better then implicit" is the way to go.

@maraujop
Collaborator

Wow, I see. I found this quote by Carl Meyer:

Note that while you can do this, it's quite likely that you'll regret it at some point (I've done it, and regretted it). It makes your templates non-portable to any other project which doesn't add_to_builtins, and it can break tests which render those templates (unless you make sure the test runner also runs add_to_builtins). All in all, it makes things more brittle for a very small gain in convenience.

It looked like a good idea, to have the tags loaded everywhere in the project. Not meaning django-crispy-forms but the project that uses it. That avoid having to repeat loads everywhere, right?

@pydanny
Collaborator

It's also a performance thing.

'Repeat' loads are a good thing, since all you are doing is adding them into templates manually. Which means those pages that don't use a particular template library aren't being throttled by Django's notoriously slow templatetag library finder (which checks every INSTALLED_APP plus the Django defaults).

Defining them in add_to_builtins means every request loads all the templatetags. Then you'll complain that Django is slow and look to node.js to increase your performance. All for the wrong reasons. :D

Ouch.

@maraujop
Collaborator

You are right, this looks as a performance eater...

I'm convinced not to use it :D I don't want to end up coding node.js no, no, no.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment