Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Templates considered" comment broken in >=0.35 #689

Closed
chrishas35 opened this issue Mar 1, 2020 · 6 comments
Closed

"Templates considered" comment broken in >=0.35 #689

chrishas35 opened this issue Mar 1, 2020 · 6 comments
Labels

Comments

@chrishas35
Copy link

chrishas35 commented Mar 1, 2020

Noticed that the "Templates Considered" comment is missing in 0.37. Believe I traced it back to #664 as you can see it in https://v0-34.datasette.io/ but not https://v0-35.datasette.io/. Looking at the template context debug between the two you can see what is missing from 0.35 vs. 0.34:

<     "datasette_version": "0.34",
<     "app_css_hash": "ffa51a",
<     "select_templates": [
<         "*index.html"
<     ],
<     "zip": "<class 'zip'>",
<     "body_scripts": [],
<     "extra_css_urls": "<generator object BaseView._asset_urls at 0x7f6529ac05f0>",
<     "extra_js_urls": "<generator object BaseView._asset_urls at 0x7f6529ac0660>",
<     "format_bytes": "<function format_bytes at 0x7f652a1588b0>",
<     "database_url": "<bound method BaseView.database_url of <datasette.views.index.IndexView object at 0x7f6529b03e50>>",
<     "database_color": "<bound method BaseView.database_color of <datasette.views.index.IndexView object at 0x7f6529b03e50>>"
---
>     "datasette_version": "0.35",
>     "database_url": "<bound method BaseView.database_url of <datasette.views.index.IndexView object at 0x7f6140dacd90>>",
>     "database_color": "<bound method BaseView.database_color of <datasette.views.index.IndexView object at 0x7f6140dacd90>>"
@simonw simonw added the bug label Mar 2, 2020
@simonw simonw changed the title template_context broken in >=0.35 "Templates considered" comment broken in >=0.35 Mar 8, 2020
@simonw
Copy link
Owner

simonw commented Apr 5, 2020

I'm going to debug this using git bisect run - the same technique I used in #716 - #716 (comment)

@simonw
Copy link
Owner

simonw commented Apr 5, 2020

Just need the one checking script to run with bisect this time:

check_templates_considered.py

import asyncio
import pathlib
from datasette.app import Datasette
import httpx


async def run_check():
    ds = Datasette([])
    async with httpx.AsyncClient(app=ds.app()) as client:
        response = await client.get("http://localhost/")
        assert 200 == response.status_code
        assert "Templates considered" in response.text


if __name__ == "__main__":
    loop = asyncio.get_event_loop()
    loop.run_until_complete(run_check())

@simonw
Copy link
Owner

simonw commented Apr 5, 2020

git bisect start master 0.34
git bisect run python check_templates_considered.py

@simonw
Copy link
Owner

simonw commented Apr 5, 2020

$ git bisect start master 0.34
Bisecting: 32 revisions left to test after this (roughly 5 steps)
[dc80e779a2e708b2685fc641df99e6aae9ad6f97] Handle scope path if it is a string
$ git bisect run python check_templates_considered.py
running python check_templates_considered.py
Traceback (most recent call last):
...
AssertionError
Bisecting: 15 revisions left to test after this (roughly 4 steps)
[7c6a9c35299f251f9abfb03fd8e85143e4361709] Better tests for prepare_connection() plugin hook, refs #678
running python check_templates_considered.py
Traceback (most recent call last):
...
AssertionError
Bisecting: 7 revisions left to test after this (roughly 3 steps)
[0091dfe3e5a3db94af8881038d3f1b8312bb857d] More reliable tie-break ordering for facet results
running python check_templates_considered.py
Traceback (most recent call last):
...
AssertionError
Bisecting: 3 revisions left to test after this (roughly 2 steps)
[ce12244037b60ba0202c814871218c1dab38d729] Release notes for 0.35
running python check_templates_considered.py
Traceback (most recent call last):
...
AssertionError
Bisecting: 1 revision left to test after this (roughly 1 step)
[70b915fb4bc214f9d064179f87671f8a378aa127] Datasette.render_template() method, closes #577
running python check_templates_considered.py
Traceback (most recent call last):
...
AssertionError
Bisecting: 0 revisions left to test after this (roughly 0 steps)
[286ed286b68793532c2a38436a08343b45cfbc91] geojson-to-sqlite
running python check_templates_considered.py
70b915fb4bc214f9d064179f87671f8a378aa127 is the first bad commit
commit 70b915fb4bc214f9d064179f87671f8a378aa127
Author: Simon Willison <swillison@gmail.com>
Date:   Tue Feb 4 12:26:17 2020 -0800

    Datasette.render_template() method, closes #577
    
    Pull request #664.

:040000 040000 def9e31252e056845609de36c66d4320dd0c47f8 da19b7f8c26d50a4c05e5a7f05220b968429725c M	datasette
bisect run success

It was 70b915f - the same bad commit that caused #716!

@simonw
Copy link
Owner

simonw commented Apr 5, 2020

Here's why: the BaseView.render() method is running jinja_env.select_template() now here:

async def render(self, templates, request, context):
template = self.ds.jinja_env.select_template(templates)
template_context = {
**context,
**{
"database_url": self.database_url,
"database_color": self.database_color,
},
}
return Response.html(
await self.ds.render_template(
template, template_context, request=request, view_name=self.name
)
)

Which means this logic is always called with a template, not a list of strings:

datasette/datasette/app.py

Lines 555 to 571 in e89b0ef

async def render_template(
self, templates, context=None, request=None, view_name=None
):
context = context or {}
if isinstance(templates, Template):
template = templates
select_templates = []
else:
if isinstance(templates, str):
templates = [templates]
template = self.jinja_env.select_template(templates)
select_templates = [
"{}{}".format(
"*" if template_name == template.name else "", template_name
)
for template_name in templates
]

@simonw
Copy link
Owner

simonw commented Apr 5, 2020

So I think the fix is to move the "select_templates": select_templates context setting bit to here instead:

async def render(self, templates, request, context):
template = self.ds.jinja_env.select_template(templates)
template_context = {
**context,
**{
"database_url": self.database_url,
"database_color": self.database_color,
},
}

@simonw simonw closed this as completed in d55fe8c Apr 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants