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

v0.54 500 error from sql query in custom template; code worked in v0.53; found a workaround #3

Closed
jrdmb opened this issue Jan 27, 2021 · 7 comments

Comments

@jrdmb
Copy link

jrdmb commented Jan 27, 2021

v0.54 500 error in sql query template; code worked in v0.53; found a workaround

schema:
CREATE TABLE "talks" ("talk" TEXT,"series" INTEGER, "talkdate" TEXT)
CREATE TABLE "series" ("id" INTEGER PRIMARY KEY, "series" TEXT, talks_list TEXT default '', website TEXT default '');

Live example of correctly rendered template in v.053: https://cosmotalks-cy6xkkbezq-uw.a.run.app/cosmotalks/talks/1

Description of problem: I needed 'sql select' code in a custom row-mydatabase-mytable.html template to lookup the series name for a foreign key integer value in the talks table. So metadata.json specifies the datasette-template-sql plugin.

The code below worked perfectly in v0.53 (just the relevant sql statement part is shown; full code is here):

{# custom addition #}  
{% for row in display_rows %}  
    ...  
    {% set sname = sql("select series from series where id = ?", [row.series]) %}  
    <strong>Series name: {{ sname[0].series }}  
    ...  
{% endfor %}  
{# End of custom addition #}  

In v0.54, that code resulted in a 500 error with a 'no such table series' message. A second query in that template also did not work but the above is fully illustrative of the problem.

All templates were up-to-date along with datasette v0.54.

Workaround: After fiddling around with trying different things, what worked was the syntax from Querying a different database from the datasette-template-sql github repo to add the database name to the sql statement:

{% set sname = sql("select series from series where id = ?", [row.series], database="cosmotalks") %}

Though this was found to work, it should not be necessary to add database="cosmotalks" since per the datasette-template-sql README, it's only needed when querying a different database, but here it's a table within the same database.

@simonw
Copy link
Owner

simonw commented Jan 28, 2021

Good catch on the workaround here. The root problem is that datasette-template-sql looks for the first available databsae if you don't provide it with a database= argument, and in Datasette 0.54 the first available database changed to being the new _internal database.

Is this a bug? I think it is - because the documented behaviour on https://docs.datasette.io/en/stable/internals.html#get-database-name is this:

name - string, optional

The name to be used for this database - this will be used in the URL path, e.g. /dbname. If not specified Datasette will pick one based on the filename or memory name.

Since the new behaviour differs from what was in the documentation I'm going to treat this as a bug and fix it.

@simonw
Copy link
Owner

simonw commented Jan 29, 2021

@simonw
Copy link
Owner

simonw commented Jan 29, 2021

I'm pretty sure this is a bug in datasette-template-sql itself - moving it there.

@simonw simonw transferred this issue from simonw/datasette Jan 29, 2021
@simonw
Copy link
Owner

simonw commented Jan 29, 2021

Running pytest after first running pip install -U datasette throws an error:

(datasette-template-sql) datasette-template-sql % pytest
========================================================================= test session starts =========================================================================
platform darwin -- Python 3.8.6, pytest-5.4.1, py-1.8.1, pluggy-0.13.1
rootdir: /Users/simon/Dropbox/Development/datasette-template-sql
plugins: asyncio-0.11.0
collected 3 items                                                                                                                                                     

tests/test_template_sql.py .F.                                                                                                                                  [100%]

============================================================================== FAILURES ===============================================================================
__________________________________________________________________ test_sql_against_default_database __________________________________________________________________

app = <datasette.utils.asgi.AsgiLifespan object at 0x110dc7280>

    @pytest.mark.asyncio
    async def test_sql_against_default_database(app):
        async with httpx.AsyncClient(app=app) as client:
            response = await client.get("http://localhost/")
            assert 200 == response.status_code
            stripped = re.sub(r"\s+", " ", response.text)
>           assert "<pre> type: table<br> name: dogs<br> </pre>" in stripped
E           assert '<pre> type: table<br> name: dogs<br> </pre>' in '<!DOCTYPE html> <html> <head> <title>Datasette: dogs, news</title> <link rel="stylesheet" href="/-/static/app.css?4e3...n ).forEach(details => details.open = false); }); </script> <!-- Templates considered: *index.html --> </body> </html>'

tests/test_template_sql.py:46: AssertionError

@simonw
Copy link
Owner

simonw commented Jan 29, 2021

Here's why:

async def execute_sql(sql, args=None, database=None):
database = database or database or next(iter(datasette.databases.keys()))
return (await datasette.execute(database, sql, args)).rows

The .get_database() method implements this logic correctly now.

@simonw simonw closed this as completed in 3afe9dc Jan 29, 2021
simonw added a commit that referenced this issue Jan 29, 2021
simonw added a commit to simonw/museums that referenced this issue Jan 29, 2021
@simonw
Copy link
Owner

simonw commented Jan 29, 2021

Fix is now deployed to https://www.niche-museums.com/ - on my laptop it exhibited the 500 error with 1.0.1 but was fixed with 1.0.2.

https://www.niche-museums.com/-/plugins

@jrdmb
Copy link
Author

jrdmb commented Jan 29, 2021

Just installed datasette-template-sql 1.0.2 and confirming on my end that the code is now working.

Many thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants