Skip to content

Conversation

russss
Copy link
Contributor

@russss russss commented Apr 29, 2019

This changeset refactors out the JSON renderer and then adds a hook and
dispatcher system to allow custom output renderers to be registered.

The CSV output renderer is untouched because supporting streaming
renderers through this system would be significantly more complex, and
probably not worthwhile.

We can't simply allow hooks to be called at request time because we need
a list of supported file extensions when the request is being routed in
order to resolve ambiguous database/table names. So, renderers need to
be registered at startup.

I've tried to make this API independent of Sanic's request/response
objects so that this can remain stable during the switch to ASGI. I'm
using dictionaries to keep it simple and to make adding additional
options in the future easy.

Fixes #440

This changeset refactors out the JSON renderer and then adds a hook and
dispatcher system to allow custom output renderers to be registered.

The CSV output renderer is untouched because supporting streaming
renderers through this system would be significantly more complex, and
probably not worthwhile.

We can't simply allow hooks to be called at request time because we need
a list of supported file extensions when the request is being routed in
order to resolve ambiguous database/table names. So, renderers need to
be registered at startup.

I've tried to make this API independent of Sanic's request/response
objects so that this can remain stable during the switch to ASGI. I'm
using dictionaries to keep it simple and to make adding additional
options in the future easy.

Fixes simonw#440
@russss russss force-pushed the pluggable_renderers branch from 404ce32 to c6c0ecd Compare April 29, 2019 18:09
@russss
Copy link
Contributor Author

russss commented Apr 29, 2019

Subsidiary note which I forgot in the commit message:

I've decided to give each view a short string name to aid in differentiating which view a hook is being called from. Since hooks are functions and not subclasses, and can get called from different places in the URL hierarchy, it's sometimes difficult to distinguish what data you're actually operating on. I think this will come in handy for other hooks as well.

@simonw
Copy link
Owner

simonw commented Apr 29, 2019

Do you have an example renderer plugin I can look at?

@russss
Copy link
Contributor Author

russss commented Apr 29, 2019

This is the minimal example (I also included it in the docs):

from datasette import hookimpl

def render_test(args, data, view_name):
    return {
       'body': 'Hello World',
       'content_type': 'text/plain'
    }

@hookimpl
def register_output_renderer():
    return {
        'extension': 'test',
        'callback': render_test
    }

I'm working on the GeoJSON one now and it should be ready soon. (I forgot I was going to run into the same problem as before - that Spatialite's stupid binary format isn't WKB and I have no way of altering the query to change that - but I've just managed to write some code to rearrange the bytes from Spatialite blob-geometry into WKB...)

@russss
Copy link
Contributor Author

russss commented Apr 29, 2019

I also just realised that I should be passing the datasette object into the hook function...as I just found I need it. So hold off merging until I've fixed that.

@russss
Copy link
Contributor Author

russss commented Apr 29, 2019

I updated the hook to pass the datasette object through now.

You can see the working GeoJSON render function here - the hook function is here.

@russss
Copy link
Contributor Author

russss commented Apr 29, 2019

Also I just pushed a change to add registered output renderers to the templates:
image

@russss
Copy link
Contributor Author

russss commented May 1, 2019

Just for the record, this PR is now finished and ready to merge from my perspective.

@simonw
Copy link
Owner

simonw commented May 1, 2019

I plan to merge this in about 7 hours time (after work).

I may tweak the plugin format a little bit before the next Datasette release though (best to merge first, make changes later I think) - for consistency with some other upcoming hooks (the facet hook in particular). I'll discuss that with you while I'm working on it.

Thanks so much for this, it's a really neat addition!

@simonw simonw merged commit cf406c0 into simonw:master May 1, 2019
russss added a commit to russss/datasette that referenced this pull request May 2, 2019
At the moment it's not easy to tell whether the hook is being called
in (for example) the row or table view, as in both cases the
`database` and `table` parameters are provided.

This passes the `view_name` added in simonw#441 to the `extra_body_script`
hook.
russss added a commit to russss/datasette that referenced this pull request May 2, 2019
At the moment it's not easy to tell whether the hook is being called
in (for example) the row or table view, as in both cases the
`database` and `table` parameters are provided.

This passes the `view_name` added in simonw#441 to the `extra_body_script`
hook.
simonw pushed a commit that referenced this pull request May 3, 2019
At the moment it's not easy to tell whether the hook is being called
in (for example) the row or table view, as in both cases the
`database` and `table` parameters are provided.

This passes the `view_name` added in #441 to the `extra_body_script`
hook.
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

Successfully merging this pull request may close these issues.

Plugin hook for additional data export formats
2 participants