1167194 - switch to django-jinja

Status: RESOLVED FIXED

(Input :: General, defect)

Description

[Will Kahn-Greene [:willkg] ET needinfo? me]

We're upgrading to Django 1.8. Django 1.8 supports jinja2 template system. This gives us a bunch of options one of which is to ditch jingo and go with something better maintained.

After a lot of discussion and some prototyping, seems like the best option is to go with django-jinja:

1. better maintained than jingo
2. it's not something we have to spend time maintaining going forward
3. works with django 1.8
4. can be configured in a way to Will's liking

This bug covers switching to django-jinja.

Comment 1

[Will Kahn-Greene [:willkg] ET needinfo? me]

Assigning this to Ricky.

Comment 2

[Ricky Rosario [:rrosario, :r1cky]]

I've been working on a "reference" project to show the way forward with jinja in django 1.8+:

https://github.com/rlr/django_jinja_ref


I ripped out all the tower/l10n stuff because that was broken and I was stuck, so this is very basic. So far we are thinking:

* Jinja2 templates go in jinja2/ folders. This means we need to rename a lot of templates/ folders when switching over from jingo.
* Jinja2 "helpers" (filters, global functions, etc) are setup in <app>/templatetags/jinja_helpers.py so that they get imported automatically by django.

I think that's the gist of it.

TODO:
* Figure out how third party apps (django-waffle) should support both jingo and django-jinja. That is mostly easy. One thing I haven't figured out with django-jinja yet is the right way to access the jinja2 env to add stuff to it. That is bug 1170160
* L10n stuff. Tower is broken horribly with django 1.8. That is bug 1167192

I have made some progress on both of those. I'll get that branch up and probably ask for feedback/help.

Comment 3

[Will Kahn-Greene [:willkg] ET needinfo? me]

Sugardough uses Django 1.8.2 and django-jinja, but doesn't have tower. Maybe we don't need most of what tower does anymore?

I added this issue to their tracker to see if anyone has thought about it:

https://github.com/mozilla/sugardough/issues/73

Comment 4

[Will Kahn-Greene [:willkg] ET needinfo? me]

This is important--making it a P2.

Comment 5

[Will Kahn-Greene [:willkg] ET needinfo? me]

Moving this to 2015q3.

Comment 6

[Ricky Rosario [:rrosario, :r1cky]]

passing this hot potato on. Here is a basic reference app I built: https://github.com/rlr/django_jinja_ref

Comment 7

[Will Kahn-Greene [:willkg] ET needinfo? me]

Grabbing this one.

I'm going to try to switch to django-jinja as a separate step before upgrading to django 1.8. I think I can do this, but it's possible I'll hit weird things. I'm pretty sure if I do hit weird things, it should be pretty obvious.

I'm pretty sure django-jinja overrides the Django extract command. Will need to keep that in mind.

Making this a 3 point bug because of all the work we've done already plus the additional work we still need to do.

Comment 8

[Will Kahn-Greene [:willkg] ET needinfo? me]

Continuing the thoughts from comment #2:

1. django-jinja has completely different configuration between Django 1.7 and Django 1.8. We have two options:

   1.1. get it all working with Django 1.7, then redo it all for Django 1.8
   1.2. do this and the Django 1.8 upgrade at the same time

I'm inclined to do everything at the same time, though that'll be a bunch of work all at once and hard to review.

2. sugardough folks had a short discussion on whether to have a jinja/ directory or change the extension for jinja files to .jinja and settled on the latter:

    https://github.com/mozilla/sugardough/issues/76

For consistency, we'll go with that. It requires renaming all our files and fixing the code so that the names are right. That's kind of a pain in the ass, but so it goes.

Comment 9

[Will Kahn-Greene [:willkg] ET needinfo? me]

I talked with Mike about .jinja vs. jinja/ and we had the following thoughts:

1. Switching from what we have now to .jinja is a lot of implementation, testing and review work whereas switching to jinja/ is easy and mostly just moving files.

2. In order to override django core templates (wouldn't want to do this) and django library templates (this comes up periodically), with jinja templates, you have to keep the same name--you can't switch the ending. Thus jinja/ is better than .jinja since the latter reduces what we're able to do. The simple examples are overriding the 403, 404 and 500 templates.

3. It's nice to name files after what they render to. e.g. Files that render to HTML end in .html and files that render to text (for example, emails) render to .txt.

4. Seems less surprising to have jinja templates in a jinja/ directory than have all the templates in the same directory tree. This might be less surprising to contributors, too.

5. We don't know of editors that require files to end in .jinja to activate a Jinja mode


Given that, I'm going to go with moving things to a jinja/ directory tree instead.

Comment 10

[Will Kahn-Greene [:willkg] ET needinfo? me]

Looked at extract/merge/gettext stuff today. When we dropped tower, we added a bunch of notes suggesting we should switch to django-jinja's makemessages which overrides Django's makemessages.

I read about Django's makemessages (https://docs.djangoproject.com/en/1.8/ref/django-admin/#makemessages) and it doesn't support Jinja2. django-jinja overrides it and adjusts some of the regexps so it does (or something like that).

I think we could get it to work, but I decided to keep the process the way it is now with extract and merge. I'll write up a bug to look into switching to makemessages later.

We're also keeping fjord.base.l10n.MozInternationalizationExtension rather than going with the jinja.ext.i18n extension. Ours collapses whitespace in msgids and also marks _() results as safe.

Comment 11

[Will Kahn-Greene [:willkg] ET needinfo? me]

In a PR: https://github.com/mozilla/fjord/pull/684

Comment 12

[Will Kahn-Greene [:willkg] ET needinfo? me]

Landed in https://github.com/mozilla/fjord/commit/5d269bd7585c21f468f13c7768352b519b2ac1f1

Pushed this to prod just now. Marking as FIXED.