New JSON design for query views #2118

simonw · 2023-07-26T23:29:21Z

WIP. Refs:

Plan for getting the new JSON format query views working #2109

📚 Documentation preview 📚: https://datasette--2118.org.readthedocs.build/en/2118/

But it breaks when it dispatches ?sql= to the new query_view function. Refs #2111, closes #2110

simonw · 2023-07-26T23:30:44Z

The _shape= stuff should use json_renderer instead - that's how the table view did it:
d97e82d#diff-5c9ef29c33ed0fde413565b23fa258d60fc3a2bb205b016db9e915c9bd5ecfb3

datasette/datasette/views/table.py

Lines 822 to 850 in c3e3ecf

    
           elif format_ in datasette.renderers.keys(): 
        
               # Dispatch request to the correct output format renderer 
        
               # (CSV is not handled here due to streaming) 
        
               result = call_with_supported_arguments( 
        
                   datasette.renderers[format_][0], 
        
                   datasette=datasette, 
        
                   columns=columns, 
        
                   rows=rows, 
        
                   sql=sql, 
        
                   query_name=None, 
        
                   database=resolved.db.name, 
        
                   table=resolved.table, 
        
                   request=request, 
        
                   view_name="table", 
        
                   # These will be deprecated in Datasette 1.0: 
        
                   args=request.args, 
        
                   data=data, 
        
               ) 
        
               if asyncio.iscoroutine(result): 
        
                   result = await result 
        
               if result is None: 
        
                   raise NotFound("No data") 
        
               if isinstance(result, dict): 
        
                   r = Response( 
        
                       body=result.get("body"), 
        
                       status=result.get("status_code") or 200, 
        
                       content_type=result.get("content_type", "text/plain"), 
        
                       headers=result.get("headers"), 
        
                   )

Instead of:

datasette/datasette/views/database.py

Lines 239 to 288 in c3e3ecf

    
           _shape = None 
        
           if "_shape" in params: 
        
               _shape = params.pop("_shape") 
        
           async def _results(_sql, _params): 
        
               # Returns (results, error (can be None)) 
        
               try: 
        
                   results = await db.execute(_sql, _params, truncate=True) 
        
                   return results, None 
        
               except Exception as e: 
        
                   return None, e 
        
           async def shape_arrays(_results): 
        
               results, error = _results 
        
               if error: 
        
                   return {"ok": False, "error": str(error)} 
        
               return { 
        
                   "ok": True, 
        
                   "rows": [list(r) for r in results.rows], 
        
                   "truncated": results.truncated, 
        
               } 
        
           async def shape_objects(_results): 
        
               results, error = _results 
        
               if error: 
        
                   return {"ok": False, "error": str(error)} 
        
               return { 
        
                   "ok": True, 
        
                   "rows": [dict(r) for r in results.rows], 
        
                   "truncated": results.truncated, 
        
               } 
        
           async def shape_array(_results): 
        
               results, error = _results 
        
               if error: 
        
                   return {"ok": False, "error": str(error)} 
        
               return [dict(r) for r in results.rows] 
        
           async def shape_arrayfirst(_results): 
        
               results, error = _results 
        
               if error: 
        
                   return {"ok": False, "error": str(error)} 
        
               return [r[0] for r in results.rows] 
        
           shape_fn = { 
        
               "arrays": shape_arrays, 
        
               "objects": shape_objects, 
        
               "array": shape_array, 
        
               "arrayfirst": shape_arrayfirst, 
        
           }[_shape or "objects"]

tests/test_api.py

simonw · 2023-08-07T20:57:46Z

17 failing tests now.

simonw · 2023-08-07T21:06:50Z

15:

=========================================================================================== short test summary info ============================================================================================
FAILED tests/test_cli.py::test_sql_errors_logged_to_stderr - assert 0 == 1
FAILED tests/test_html.py::test_show_hide_sql_query - AssertionError: assert False
FAILED tests/test_canned_queries.py::test_magic_parameters_cannot_be_used_in_arbitrary_queries - assert 400 == 200
FAILED tests/test_html.py::test_templates_considered[/fixtures-database-fixtures.html, *database.html] - assert '<!-- Templates considered: database-fixtures.html, *database.html -->' in '<!DOCTYPE html>\n<html>\n<head>\n    <title>fixtures</title>\n    <link rel="stylesheet" href="/-/static/app.css?d599......
FAILED tests/test_html.py::test_query_json_csv_export_links - assert '<a href="/fixtures.csv?sql=select+1&amp;_size=max">CSV</a>' in '<!DOCTYPE html>\n<html>\n<head>\n    <title>fixtures: select 1</title>\n    <link rel="stylesheet" href="/-/static/ap...se"}, "view...
FAILED tests/test_html.py::test_base_url_config[True-/fixtures?sql=select+1] - AssertionError: {
FAILED tests/test_api.py::test_cors[/fixtures.json?sql=select+blah-400] - assert 200 == 400
FAILED tests/test_html.py::test_alternate_url_json[/fixtures?sql=select+*+from+facetable-http://localhost/fixtures.json?sql=select+*+from+facetable] - assert '<link rel="alternate" type="application/json+datasette" href="http://localhost/fixtures.json?sql=select+*+from+facetable">' in '<!DOCTYPE html>\n<html>\n<head>\n    <title>fixtures: select * from...
FAILED tests/test_plugins.py::test_hook_extra_css_urls[/fixtures-expected_decoded_object1] - AssertionError: assert equals failed
FAILED tests/test_plugins.py::test_view_names[/fixtures-database] - AssertionError: assert equals failed
FAILED tests/test_plugins.py::test_view_names[/fixtures?sql=select+1-database] - AssertionError: assert equals failed
FAILED tests/test_plugins.py::test_hook_extra_body_script[/fixtures-expected_extra_body_script1] - AssertionError: assert equals failed
FAILED tests/test_html.py::test_base_url_config[False-/fixtures?sql=select+1] - AssertionError: {
FAILED tests/test_table_api.py::test_max_returned_rows - KeyError: 'query'
FAILED tests/test_html.py::test_alternate_url_json[/fixtures-http://localhost/fixtures.json] - KeyError: 'link'
============================================================================ 15 failed, 1297 passed, 2 skipped, 1 xfailed in 58.15s ============================================================================

simonw · 2023-08-07T23:11:04Z

One of the failing tests is because this URL on my local environment returns this:

http://127.0.0.1:8001/_memory.json?sql=select+blah

{"rows": [], "columns": [], "truncated": false}

With a 200 status code. It should be an error.

https://latest.datasette.io/_memory.json?sql=select+blah returns a 400 status code with the following JSON:

{
  "ok": false,
  "database": "_memory",
  "query_name": null,
  "rows": [],
  "truncated": false,
  "columns": [],
  "query": {
    "sql": "select blah",
    "params": {}
  },
  "error": "no such column: blah",
  "private": false,
  "allow_execute_sql": true,
  "query_ms": 2.68310698447749,
  "source": "tests/fixtures.py",
  "source_url": "https://github.com/simonw/datasette/blob/main/tests/fixtures.py",
  "license": "Apache License 2.0",
  "license_url": "https://github.com/simonw/datasette/blob/main/LICENSE"
}

So I need a design for what this should look like for the error case.

I should consider what errors look like for alternative formats too.

simonw · 2023-08-07T23:12:37Z

https://latest.datasette.io/_memory.csv?sql=select+blah is a blank page right now:

curl -I 'https://latest.datasette.io/_memory.csv?sql=select+blah'

HTTP/2 200 
access-control-allow-origin: *
access-control-allow-headers: Authorization, Content-Type
access-control-expose-headers: Link
access-control-allow-methods: GET, POST, HEAD, OPTIONS
access-control-max-age: 3600
content-type: text/plain; charset=utf-8
x-databases: _memory, _internal, fixtures, fixtures2, extra_database, ephemeral
date: Mon, 07 Aug 2023 23:12:15 GMT
server: Google Frontend

simonw · 2023-08-07T23:15:14Z

New error design:

{"ok": false, "error": "Error message here", "rows": [], "columns": [], "truncated": false}

I guess that means I need to add "ok": true to the default format as well - which makes sense because table JSON has that already:

https://latest.datasette.io/fixtures/facetable.json

{
  "ok": true,
  "next": null,
  "rows": [...]
}

simonw · 2023-08-07T23:50:26Z

This bit needs some more work:

datasette/datasette/views/database.py

Lines 514 to 518 in a791115

    
           url_csv=datasette.urls.path( 
        
               path_with_format( 
        
                   request=request, format="csv" 
        
               )  # , extra_qs=url_csv_args) 
        
           ),

The original logic for that is from here, I need to figure out how to replicate it:

datasette/datasette/views/base.py

Line 368 in 5139c08

url_csv_args = {"_size": "max", **url_labels_extra}

datasette/datasette/views/base.py

Lines 342 to 344 in 5139c08

    
           url_labels_extra = {} 
        
           if data.get("expandable_columns"): 
        
               url_labels_extra = {"_labels": "on"}

UPDATE: Fixed the _size=max bit, but I've not replicated the behaviour where it adds ?_labels=on if there are expandable columns yet.

simonw · 2023-08-07T23:58:21Z

11 tests left:

FAILED tests/test_html.py::test_alternate_url_json[/fixtures-http://localhost/fixtures.json] - KeyError: 'link'
FAILED tests/test_html.py::test_alternate_url_json[/fixtures?sql=select+*+from+facetable-http://localhost/fixtures.json?sql=select+*+from+facetable] - assert '<link rel="alternate" type="application/json+datasette" href="http://localhost/fixtures.json?sql=select+*+from+facetable">' in '<!DOCTYPE html>\n<html>\n<head>\n    <title>fixtures: select * from...
FAILED tests/test_html.py::test_query_page_truncates - assert equals failed
FAILED tests/test_html.py::test_templates_considered[/fixtures-database-fixtures.html, *database.html] - assert '<!-- Templates considered: database-fixtures.html, *database.html -->' in '<!DOCTYPE html>\n<html>\n<head>\n    <title>fixtures</title>\n    <link rel="stylesheet" href="/-/static/app.css?d599......
FAILED tests/test_html.py::test_query_json_csv_export_links - assert '<a href="/fixtures.csv?sql=select+1&amp;_size=max">CSV</a>' in '<!DOCTYPE html>\n<html>\n<head>\n    <title>fixtures: select 1</title>\n    <link rel="stylesheet" href="/-/static/ap...se"}, "view...
FAILED tests/test_plugins.py::test_hook_extra_css_urls[/fixtures-expected_decoded_object1] - AssertionError: assert equals failed
FAILED tests/test_api.py::test_cors[/fixtures.json?sql=select+blah-400] - KeyError: 'Access-Control-Allow-Origin'
FAILED tests/test_plugins.py::test_hook_extra_body_script[/fixtures-expected_extra_body_script1] - AssertionError: assert equals failed
FAILED tests/test_table_api.py::test_max_returned_rows - KeyError: 'query'
FAILED tests/test_plugins.py::test_view_names[/fixtures-database] - AssertionError: assert equals failed
FAILED tests/test_plugins.py::test_view_names[/fixtures?sql=select+1-database] - AssertionError: assert equals failed
======================================================================= 11 failed, 1301 passed, 2 skipped, 1 xfailed in 64.10s (0:01:04) =======================================================================

simonw · 2023-08-08T01:34:42Z

3 failures left:

=================================================================== short test summary info ====================================================================
FAILED tests/test_html.py::test_query_json_csv_export_links - assert '<a href="/fixtures.csv?sql=select+1&amp;_size=max">CSV</a>' in '<!DOCTYPE html>\n<html>\n<head>\n    <title>fixtures: select 1</title>\n    <link r...
FAILED tests/test_api.py::test_cors[/fixtures.json?sql=select+blah-400] - KeyError: 'Access-Control-Allow-Origin'
FAILED tests/test_table_api.py::test_max_returned_rows - KeyError: 'query'
=============================================== 3 failed, 1309 passed, 2 skipped, 1 xfailed in 61.34s (0:01:01) ================================================

simonw · 2023-08-08T01:42:55Z

UPDATE: Fixed the _size=max bit, but I've not replicated the behaviour where it adds ?_labels=on if there are expandable columns yet.

It looks like that behaviour is only relevant to table views, and it's already implemented - https://latest.datasette.io/fixtures/roadside_attraction_characteristics links to https://latest.datasette.io/fixtures/roadside_attraction_characteristics.csv?_labels=on&_size=max

simonw · 2023-08-08T01:46:12Z

I'm going to squash merge this even though it's a bit big, because I want a clean passing commit in the main branch.

simonw added 2 commits July 26, 2023 12:53

New database_view() works for HTML and JSON

2e40a50

But it breaks when it dispatches ?sql= to the new query_view function. Refs #2111, closes #2110

New query_view returning JSON

c3e3ecf

simonw added the refactor label Jul 26, 2023

simonw added this to the SQL query views with new JSON format milestone Jul 26, 2023

simonw added 6 commits July 26, 2023 16:46

WIP

ff728cc

New Context dataclass/subclass mechanism, refs #2127

002289b

Define QueryContext and extract get_tables() method, refs #2127

2f9038a

First version that uses the QueryContext context, refs #2127

743c13c

Handle DB errors

dff29b0

Down to 31 failing tests

2dd4e14

simonw commented Aug 7, 2023

View reviewed changes

tests/test_api.py Outdated Show resolved Hide resolved

simonw added 4 commits August 7, 2023 11:20

Show error messages with a form to edit the SQL

c138df6

Fix OPTIONS bug by porting DatbaseView to be a View subclass

e2b60f5

Expose async_view_for_class.view_class for test_routes test

f2a110f

Fix for messages tests

80953da

Added table_columns schema back to query/database pages

fcdb96c

Clean up imports a bit

8ec6bc8

simonw mentioned this pull request Aug 7, 2023

CSV ?sql= should indicate errors #2129

Open

simonw mentioned this pull request Aug 7, 2023

Render plugin mechanism needs error and truncated fields #2130

Closed

simonw added 2 commits August 7, 2023 16:36

error/truncated aruments for renderers, refs #2130

cd1dd01

Implemented show/hide links

a791115

Fixed a test and implemented show_hide_hidden

ea24c9c

simonw added 3 commits August 7, 2023 18:18

Implemented alternate_url_json

8fbcd7f

select_templates for templates considered comment

7532feb

Set view_name correctly in a few more places

b45965b

simonw added 3 commits August 7, 2023 18:36

More cors headers, plus deleted some dead code

0505032

Fixed broken test

5382ec6

CSV links have _size=max on them

8b1dea3

simonw marked this pull request as ready for review August 8, 2023 01:45

simonw merged commit 1377a29 into main Aug 8, 2023
15 checks passed

simonw deleted the new-json-query-views branch August 8, 2023 01:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New JSON design for query views #2118

New JSON design for query views #2118

simonw commented Jul 26, 2023 •

edited by github-actions bot

simonw commented Jul 26, 2023

simonw commented Aug 7, 2023

simonw commented Aug 7, 2023

simonw commented Aug 7, 2023 •

edited

simonw commented Aug 7, 2023

simonw commented Aug 7, 2023 •

edited

simonw commented Aug 7, 2023 •

edited

simonw commented Aug 7, 2023

simonw commented Aug 8, 2023

simonw commented Aug 8, 2023

simonw commented Aug 8, 2023

New JSON design for query views #2118

New JSON design for query views #2118

Conversation

simonw commented Jul 26, 2023 • edited by github-actions bot

simonw commented Jul 26, 2023

simonw commented Aug 7, 2023

simonw commented Aug 7, 2023

simonw commented Aug 7, 2023 • edited

simonw commented Aug 7, 2023

simonw commented Aug 7, 2023 • edited

simonw commented Aug 7, 2023 • edited

simonw commented Aug 7, 2023

simonw commented Aug 8, 2023

simonw commented Aug 8, 2023

simonw commented Aug 8, 2023

simonw commented Jul 26, 2023 •

edited by github-actions bot

simonw commented Aug 7, 2023 •

edited

simonw commented Aug 7, 2023 •

edited

simonw commented Aug 7, 2023 •

edited