Remove default django template loaders.

[mtyaka]

Mako filesystem/app_directories loaders already wrap default django template loaders.

Mako loaders delegate the load_template_source method to the base loader that they wrap, so there's no reason to explicitly include the two django loaders in the settings.

Partner information: 3rd party-hosted open edX instance
JIRA ticket: https://openedx.atlassian.net/browse/OSPR-1084
Sandbox URLs: TBD

[openedx-webhooks]

Thanks for the pull request, @mtyaka! I've created OSPR-1084 to keep track of it in JIRA. JIRA is a place for product owners to prioritize feature reviews by the engineering development teams.

Feel free to add as much of the following information to the ticket:

• supporting documentation
• edx-code email threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will still be done via the GitHub pull request interface. As a reminder, our process documentation is here.

[Kelketek]

👍

[nedbat]

@mtyaka sorry for my ignorance, but I notice that cms settings don't include the Mako loaders. Do we not use Mako in Studio?? (I know this sounds like a really newb question...)

[nedbat]

We do use Mako all over Studio. How is it being loaded? Why don't we need to make similar changes in "cms"?

[mtyaka]

@nedbat Yeah, that is a bit confusing. It took me a while to figure out how template rendering works in edx-platform.

Most of the time, we render main mako templates using edxmako.shortcuts.render_to_response. This completely bypasses django template loaders defined in TEMPLATES['OPTIONS']['loaders']. The only time these loaders are used is when rendering standard django (not mako) templates, and when either django or mako templates are included (with {% include '...' %}) inside standard django templates.

Some parts of the LMS (most notably the course wiki) use the main_django.html template, which is not a mako template, but it does {% include %} some mako templates. That's the only reason edxmako.makoloader.MakoFilesystemLoader and edxmako.makoloader.MakoAppDirectoriesLoader are needed - to be able to include mako templates inside django templates.

I guess in the CMS we never do that (include mako templates in django templates). We render mako templates directly using render_to_response. Some third party apps (for example debug tololbar) use django templates, but they never include mako templates, which is why in the CMS we don't have to use MakoFilesystemLoader/MakoAppDirectoriesLoader, but can use standard django filesystem.Loader/app_directories.Loader loaders instead.

Note that we could easily replace filesystem.Loader/app_directories.Loader in the CMS with MakoFilesystemLoader/MakoAppDirectoriesLoader and everything would continue to work exactly the same.

[nedbat]

Could you add a comment about this in the settings where we set up the Mako loaders? Then we are good to go.

[mtyaka]

@nedbat I added some comments to the settings and to the MakoLoader docstring.

[bradenmacdonald]

@nedbat It sounds like you are happy with this - can we get your formal +1? And/or are we waiting for further review?

[mattdrayer]

@mtyaka @nedbat @Kelketek -- sorry I am just now jumping in on this PR so forgive me while I get up to speed. Overall the change appears docile, however I am wondering if this is simply a code cleanup exercise or if the system's behavior is actually being affected by the change? It seems we are just doing some cleanup here? @ziafazal @saleem-latif, FYI

[mtyaka]

@mattdrayer Correct, this is just code cleanup, it should not have any effect on the way the system behaves.

[mattdrayer]

👍

[nedbat]

👍 Sorry for the delay.

[mtyaka]

Thanks!