home / github / issue_comments

Menu
  • Search all tables
  • GraphQL API

issue_comments: 502393107

This data as json

html_url issue_url id node_id user created_at updated_at author_association body reactions issue performed_via_github_app
https://github.com/simonw/datasette/issues/272#issuecomment-502393107 https://api.github.com/repos/simonw/datasette/issues/272 502393107 MDEyOklzc3VlQ29tbWVudDUwMjM5MzEwNw== 9599 2019-06-15T19:25:54Z 2019-06-19T01:20:14Z OWNER

OK, time for a solid implementation plan.

As soon as https://github.com/django/asgiref/pull/92 is merged (hopefully very soon) the ASGI spec will have support for an optional raw_path - which means we can continue to use table%2Fnames with embedded / without being unable to tell if a path has been decoded or not.

Steps to implement:

Refactor classes, then add .asgi() method to BaseView

Add a .asgi(self, scope, receive, send) method to my base view class. This will expose an ASGI interface to the outside world: the method itself will construct a request object and call the existing .get() method.

My only true shared base class is actually RenderMixin because the IndexView doesn't extend BaseView. I'm going to refactor the class hierarchy a bit here - AsgiView will be my top level class with the .asgi() method on it. RenderMixin will be renamed BaseView(AsgiView), while existing BaseView will be renamed DataView(BaseView) since it mainly exists to introduce the handy .data() abstraction.

So...

  • AsgiView - has .asgi() method, extends Sanic HTTPMethodView (for the moment)
  • BaseView(AsgiView) - defines utility methods currently on RenderMixin
  • IndexView(BaseView) - the current IndexView
  • DataView(BaseView) - defines the utilities currently on BaseView, including data()
  • Everything else subclasses DataView

Extract routing logic out into a new DatasetteView

I considered calling this RouteView, but one of the goals of this project is to allow other ASGI apps to import Datasette itself and reuse it as its own ASGI function.

So DatasetteView will subclass BaseView and will do all of the routing logic. That logic currently lives here:

https://github.com/simonw/datasette/blob/aa911122feab13f8e65875c98edb00fd3832b7b8/datasette/app.py#L594-L640

For tests: Implement a version of app_client.get() that calls ASGI instead

Almost all of the unit tests currently use app_client.get("/path..."). I want to be able to run tests against both ASGI and existing-Sanic, so for the moment I'm going to teach app_client.get() to use ASGI instead but only in the presence of a new environment variable. I can then have Travis run the tests twice - once with that environement variable and once without.

Make datasette serve --asgi run ASGI and uvicorn

Uvicorn supports Python 3.5 again as of https://github.com/encode/uvicorn/issues/330 - so it's going to be the new dependency for Datasette.

Do some comparative testing of ASGI and non-ASGI

Just some sanity checking to make sure there aren't any weird issues.

Final step: refactor out Sanic

Hopefully this will just involve changes being made to the AsgiView base class, since that subclasses Sanic HTTPMethodView.

Bonus: It looks like dropping Sanic as a dependency in favour of Uvicorn should give us Windows support! https://github.com/encode/uvicorn/issues/82

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
324188953  
Powered by Datasette · Queries took 1.264ms · About: github-to-sqlite