-
-
Notifications
You must be signed in to change notification settings - Fork 51
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
ASGI Support #532
Merged
Merged
ASGI Support #532
Changes from 52 commits
Commits
Show all changes
101 commits
Select commit
Hold shift + click to select a range
5eaf1a1
Trash
dmanchon 853f476
requirements
dmanchon d023abb
Tests + changes to factory
masipcat 3bd33e9
hyper
dmanchon ea8c6dd
Clean a bit
dmanchon 4acf812
make hypercorn work
masipcat f59e459
Using custom Request instead of aiohttp.Request
masipcat b11dd0d
POST fixed. Molotov test
dmanchon 032f68c
Fix testclient fixture
masipcat ab8a126
Small changes
masipcat 898142b
websockets with asgi draft
dmanchon f010e3a
websocket close handling
dmanchon e2c38c6
Move requests implementations to a module
dmanchon 1827366
fix test
vangheem 55a66d8
Fixed GuillotinaRequest.query_string
masipcat 62fff01
Checkpoint. Almost all tests are green
masipcat 093e94f
Fix ws tests
dmanchon d8d6a13
Implement IRequest record method on asgi
dmanchon 27e4b2a
Deleted aiohttp request
masipcat db9b798
Fix debug headers handler
dmanchon 5ae7797
Merge branch 'master' into asgi-support
masipcat 89fafb3
Merge branch 'asgi-support' of github.com:plone/guillotina into asgi-…
masipcat 05d10ef
Fix deleted ws import
masipcat 309dce8
Fixed AsgiStreamReader bug
masipcat 2fc67b2
More fixes
masipcat 9c944af
Small fix
dmanchon df15f81
Simulate incoming request in chunks for uploads
dmanchon 826dd32
Remove starlette dep
dmanchon 29dd329
flake8
dmanchon d72d373
Almost there
masipcat 1c4d5c7
All tests green
masipcat 69adf04
Now should be all green
masipcat 7952fb0
Trying to fix failing tests in travis
masipcat 7c10a56
I hope now is fixed...
masipcat 76963a7
Small fix
masipcat 5269254
Revert "Trying to fix failing tests in travis"
masipcat 49ab931
Fix tests when runnign with DB_SCHEMA != public
masipcat c62bd3a
update to last asgi test client
dmanchon 2a0146e
Updated async-asgi-testclient
masipcat 223f083
Merge branch 'master' into asgi-support
masipcat 9eec6ec
Small changes
masipcat eee7568
Fix merge
masipcat 70a2a0f
Merge branch 'master' into asgi-support
masipcat ed3e7eb
Configured pg v10 in pytest-docker-fixtures + small change in fixtures
masipcat 99e012b
Merge branch 'master' into asgi-support
masipcat f19412a
Merge branch 'master' into asgi-support
masipcat 1c6dca7
Clean up unused methods
masipcat 841333d
Merge branch 'master' into asgi-support
masipcat 8ecb6b8
Rearrenged code and reduced code that depends on aiohttp
masipcat 3c7eb26
Merge branch 'master' into asgi-support
masipcat efe666b
Updated changelog
masipcat 4f0714e
Fix pg catalog tests
masipcat c8fe97c
Remove aiohttp dependecy for websockets
dmanchon dcba06c
Remove aiohttp dependecy for websockets
dmanchon 7aafe6f
Remove aiohttp dependecy for request
dmanchon 30b58fa
Flake8
dmanchon 8fe4e10
Replaced 'loop' fixture from 'aiohttp' for 'event_loop' from 'pytest-…
masipcat ca8be11
Merge branch 'master' into asgi-support
masipcat 2086715
Fix cockroach fixture
masipcat f6b49ba
Reduced aiohttp dependence. TODO: traversal/router and CORS
masipcat 147c233
BOOM! Merge branch 'master' into asgi-support
masipcat fbb0b76
Lot of fixes
masipcat b375401
Fixed flake8 and mypy
masipcat 133a30e
black
masipcat f7f6669
Added some tests and cleaned unused code
masipcat 22c6a8a
Mypy
masipcat 00eaacd
Asgi support: no aiohttp (#654)
vangheem 699176e
Merge branch 'master' into asgi-support
masipcat 168c560
Merge branch 'master' into asgi-support
masipcat 80f25f1
isort
masipcat f73bef1
Merge branch 'master' into asgi-support
masipcat 98f4c11
Merge branch 'master' into asgi-support
masipcat fbc08df
Merge branch 'master' into asgi-support
masipcat f1211c8
Documented how to use differents ASGI servers + small changes
masipcat 272f331
Small fixes
masipcat 307a31d
Merge branch 'master' into asgi-support
masipcat 095125f
mypy-flake8
masipcat 6726ce1
Merge branch 'asgi-support' of github.com:plone/guillotina into asgi-…
masipcat 3203b0d
Changes and fixes
masipcat e5d6a90
Removed yarl
masipcat 0265be1
Support for middlewares
masipcat e0955ef
Merge branch 'master' into asgi-support
masipcat 0333e34
Black
masipcat 61d7d20
Merge branch 'master' into asgi-support
masipcat 49230f1
Updated Cython for python3.8 (required by uvloop)
masipcat 515327a
fix uvloop
masipcat ed069ad
requested changes
masipcat 12deec6
Merge branch 'master' into asgi-support
masipcat dc73ece
Removed python 3.8 in travis
masipcat f925322
Merge branch 'master' into asgi-support
masipcat b57cd01
Changed implementation of reify
masipcat 88751c5
Merge branch 'master' into asgi-support
masipcat 7ae3546
Merge branch 'master' into asgi-support
masipcat c94cd49
Fix some tests are skiped and mypy errors
masipcat 6295819
Install extra 'testdata' in travis
masipcat a82df48
Merge branch 'master' into asgi-support
masipcat be427dd
Fixed tests
masipcat 5547069
Merge branch 'master' into asgi-support
masipcat a7bedb5
Merge branch 'master' into asgi-support
masipcat 827e228
Small fixes
masipcat 52f4200
fix
masipcat File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,115 @@ | ||
from guillotina import glogging | ||
from guillotina import task_vars | ||
from guillotina.exceptions import ConflictError | ||
from guillotina.exceptions import TIDConflictError | ||
from guillotina.request import AsgiStreamReader | ||
from guillotina.request import Request | ||
|
||
|
||
logger = glogging.getLogger('guillotina') | ||
|
||
|
||
def headers_to_list(headers): | ||
return [[k.encode(), v.encode()] for k, v in headers.items()] | ||
|
||
|
||
class AsgiApp: | ||
def __init__(self, config_file, settings, loop): | ||
self.app = None | ||
self.config_file = config_file | ||
self.settings = settings | ||
self.loop = loop | ||
self.on_cleanup = [] | ||
self.route = None | ||
|
||
async def __call__(self, scope, receive, send): | ||
if scope["type"] == "http" or scope["type"] == "websocket": | ||
return await self.handler(scope, receive, send) | ||
|
||
elif scope["type"] == "lifespan": | ||
while True: | ||
message = await receive() | ||
if message['type'] == 'lifespan.startup': | ||
await self.startup() | ||
await send({'type': 'lifespan.startup.complete'}) | ||
elif message['type'] == 'lifespan.shutdown': | ||
await self.shutdown() | ||
await send({'type': 'lifespan.shutdown.complete'}) | ||
return | ||
|
||
async def startup(self): | ||
from guillotina.factory.app import startup_app | ||
self.app = await startup_app( | ||
config_file=self.config_file, | ||
settings=self.settings, | ||
loop=self.loop, | ||
server_app=self | ||
) | ||
return self.app | ||
|
||
async def shutdown(self): | ||
for clean in self.on_cleanup: | ||
await clean(self) | ||
|
||
async def handler(self, scope, receive, send): | ||
# Aiohttp compatible StreamReader | ||
payload = AsgiStreamReader(receive) | ||
|
||
if scope["type"] == "websocket": | ||
scope["method"] = "GET" | ||
|
||
request = Request( | ||
scope["scheme"], | ||
scope["method"], | ||
scope["path"], | ||
scope["query_string"], | ||
scope["headers"], | ||
payload, | ||
loop=self.loop, | ||
send=send, | ||
scope=scope, | ||
receive=receive | ||
) | ||
|
||
task_vars.request.set(request) | ||
resp = await self.request_handler(request) | ||
|
||
# WS handling after view execution missing here!!! | ||
if scope["type"] != "websocket": | ||
from guillotina.response import StreamResponse | ||
|
||
if not isinstance(resp, StreamResponse): | ||
await send( | ||
{ | ||
"type": "http.response.start", | ||
"status": resp.status, | ||
"headers": headers_to_list(resp.headers) | ||
} | ||
) | ||
body = resp.text or "" | ||
await send({"type": "http.response.body", "body": body.encode()}) | ||
|
||
async def request_handler(self, request, retries=0): | ||
try: | ||
route = await self.app.router.resolve(request) | ||
return await route.handler(request) | ||
|
||
except (ConflictError, TIDConflictError) as e: | ||
if self.settings.get('conflict_retry_attempts', 3) > retries: | ||
label = 'DB Conflict detected' | ||
if isinstance(e, TIDConflictError): | ||
label = 'TID Conflict Error detected' | ||
tid = getattr(getattr(request, '_txn', None), '_tid', 'not issued') | ||
logger.debug( | ||
f'{label}, retrying request, tid: {tid}, retries: {retries + 1})', | ||
exc_info=True) | ||
request._retry_attempt = retries + 1 | ||
request.clear_futures() | ||
return await self.request_handler(request, retries + 1) | ||
else: | ||
logger.error( | ||
'Exhausted retry attempts for conflict error on tid: {}'.format( | ||
getattr(getattr(request, '_txn', None), '_tid', 'not issued') | ||
)) | ||
from guillotina.response import HTTPConflict | ||
return HTTPConflict() |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we default with uvicorn but I guess we'll need docs/info on how to run guillotina with uvicorn/etc by the normal startup command for those
uvicorn guillotina:app_module_thingy
if possible.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we have these options (options are written in order of preference (desc)):
serve = ServerCommand.run
inguillotina/__init__.py
so any ASGI server can start guillotina withG_CONFIG=./config.json uvicorn guillotina:serve
(we can't pass custom arguments to our app when calling with uvicorn or any other server). With this approach the propertieshost
andport
won't be configurable from theconfig.json
.ServerCommand
a list of ASGI servers, so we can launch the server withg -c config.json serve --uvicorn
org -c config.json serve --hypercorn --host 0.0.0.0 --port 8080
(thehost
andport
could be read from the config as well). But this would required do something like:get_settings()
andmake_app(settings=)
and delegate to the user this logic. For example, by doingapp = make_app(settings=get_settings(file="config.json"))
inapp.py
, and then running the server withuvicorn app:app
or by calling the server from code, likeif __name__ == "__main__": uvicorn.run(app, **params)
and running the file withpython app.py
I'm not sure about the third option, could be tricky to implement it.
Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we do a combination of 1 and 2?
#2
would include a default server to run with(say uvicorn) ootb but then you can also override it with maybe--asgi-server=module.path.to.runner
.#2
would then depend on asgi providing a generic way to hook into for this--I'm not sure there isn't one ootb.I think I would at the very least like an ootb implementation where the
serve
command worked with a default supported/tested implementation.Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean something like
--asgi-server=uvicorn.run
, right? In that case, there isn't a standard way to call an asgi server from code.uvicorn
:hypercorn
:daphne
:# Doesn't provide documentation on how to run the server from code
I think we can provide an ootb implementation that works with
uvicorn
andhypercorn
, and the user can select which server wants to run with--asgi-server=hypercorn
(default touvicorn
). Something like:What do you think?
PS: I'm not sure I understood what would be "a combination of [options] 1 and 2"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I'm good with
--asgi-server
.Maybe we can make sure to have good docs on creating app object to run with any server manually as well so users can integrate with another server before we have support for it. (or if they just want to have a different startup implementation)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done a first implementation. Suggestions are welcome :-)