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 |