-
Notifications
You must be signed in to change notification settings - Fork 21
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
async client #50
Merged
Merged
async client #50
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
bcb9560
add async client
chadawagner 53c38c9
create/close single aiohttp session
chadawagner e648d9e
update template
chadawagner 6d270a3
update example
chadawagner f54d2fd
fix bug in missing content-type exception
chadawagner 814dff4
session param in init, better teardown and examples
chadawagner 744006e
formatting
chadawagner 15daba4
don't auto-create client session
chadawagner 183e5ff
update example
chadawagner 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,86 @@ | ||
try: | ||
import asyncio | ||
import aiohttp | ||
except ModuleNotFoundError: | ||
pass | ||
|
||
from twirp.context import Context | ||
from twirp.exceptions import TwirpServerException | ||
|
||
from generated import haberdasher_twirp, haberdasher_pb2 | ||
|
||
client = haberdasher_twirp.HaberdasherClient("http://localhost:3000") | ||
|
||
try: | ||
response = client.MakeHat( | ||
ctx=Context(), request=haberdasher_pb2.Size(inches=12), server_path_prefix="/twirpy") | ||
if not response.HasField('name'): | ||
print("We didn't get a name!") | ||
print(response) | ||
except TwirpServerException as e: | ||
print(e.code, e.message, e.meta, e.to_dict()) | ||
server_url = "http://localhost:3000" | ||
timeout_s = 5 | ||
|
||
|
||
def main(): | ||
client = haberdasher_twirp.HaberdasherClient(server_url, timeout_s) | ||
|
||
try: | ||
response = client.MakeHat( | ||
ctx=Context(), | ||
request=haberdasher_pb2.Size(inches=12), | ||
server_path_prefix="/twirpy", | ||
) | ||
if not response.HasField("name"): | ||
print("We didn't get a name!") | ||
print(response) | ||
except TwirpServerException as e: | ||
print(e.code, e.message, e.meta, e.to_dict()) | ||
|
||
|
||
async def async_main(): | ||
client = haberdasher_twirp.AsyncHaberdasherClient(server_url, timeout_s) | ||
|
||
try: | ||
response = await client.MakeHat( | ||
ctx=Context(), | ||
request=haberdasher_pb2.Size(inches=12), | ||
server_path_prefix="/twirpy", | ||
) | ||
if not response.HasField("name"): | ||
print("We didn't get a name!") | ||
print(response) | ||
except TwirpServerException as e: | ||
print(e.code, e.message, e.meta, e.to_dict()) | ||
|
||
|
||
async def async_with_session(): | ||
# It is optional but recommended to provide your own ClientSession to the twirp client | ||
# either on init or per request, and ensure it is closed properly on app shutdown. | ||
# Otherwise, the client will create its own session to use, which it will attempt to | ||
# close in its __del__ method, but has no control over how or when that will get called. | ||
|
||
# NOTE: ClientSession may only be created (or closed) within a coroutine. | ||
session = aiohttp.ClientSession( | ||
server_url, timeout=aiohttp.ClientTimeout(total=timeout_s) | ||
) | ||
|
||
# If session is provided, session controls the timeout. Timeout parameter to client init is unused | ||
client = haberdasher_twirp.AsyncHaberdasherClient(server_url, session=session) | ||
|
||
try: | ||
response = await client.MakeHat( | ||
ctx=Context(), | ||
request=haberdasher_pb2.Size(inches=12), | ||
server_path_prefix="/twirpy", | ||
# Optionally provide a session per request | ||
# session=session, | ||
) | ||
if not response.HasField("name"): | ||
print("We didn't get a name!") | ||
print(response) | ||
except TwirpServerException as e: | ||
print(e.code, e.message, e.meta, e.to_dict()) | ||
finally: | ||
# Close the session (could also use a context manager) | ||
await session.close() | ||
|
||
|
||
if __name__ == "__main__": | ||
if hasattr(haberdasher_twirp, "AsyncHaberdasherClient"): | ||
print("using async client") | ||
asyncio.run(async_main()) | ||
else: | ||
main() |
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 |
---|---|---|
@@ -1,2 +1,2 @@ | ||
twirp==0.0.2 | ||
uvicorn==0.12.2 | ||
twirp==0.0.8 | ||
uvicorn==0.23.2 |
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 |
---|---|---|
|
@@ -18,6 +18,9 @@ | |
'structlog', | ||
'protobuf' | ||
], | ||
extras_require={ | ||
'async': ['aiohttp'], | ||
}, | ||
test_requires=[ | ||
], | ||
zip_safe=False) |
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,69 @@ | ||
import asyncio | ||
import json | ||
import aiohttp | ||
|
||
from . import exceptions | ||
from . import errors | ||
|
||
|
||
class AsyncTwirpClient: | ||
def __init__(self, address, timeout=5, session=None): | ||
self._address = address | ||
self._timeout = timeout | ||
self._session = session | ||
self._should_close_session = False | ||
|
||
def __del__(self): | ||
if self._should_close_session: | ||
try: | ||
loop = asyncio.get_event_loop() | ||
if loop.is_running(): | ||
loop.create_task(self._session.close()) | ||
elif not loop.is_closed(): | ||
loop.run_until_complete(self._session.close()) | ||
except RuntimeError: | ||
pass | ||
|
||
@property | ||
def session(self): | ||
if self._session is None: | ||
self._session = aiohttp.ClientSession( | ||
self._address, timeout=aiohttp.ClientTimeout(total=self._timeout) | ||
) | ||
self._should_close_session = True | ||
return self._session | ||
|
||
async def _make_request( | ||
self, *, url, ctx, request, response_obj, session=None, **kwargs | ||
): | ||
headers = ctx.get_headers() | ||
if "headers" in kwargs: | ||
headers.update(kwargs["headers"]) | ||
kwargs["headers"] = headers | ||
kwargs["headers"]["Content-Type"] = "application/protobuf" | ||
try: | ||
async with await (session or self.session).post( | ||
url=url, data=request.SerializeToString(), **kwargs | ||
) as resp: | ||
if resp.status == 200: | ||
response = response_obj() | ||
response.ParseFromString(await resp.read()) | ||
return response | ||
try: | ||
raise exceptions.TwirpServerException.from_json(await resp.json()) | ||
except (aiohttp.ContentTypeError, json.JSONDecodeError): | ||
raise exceptions.twirp_error_from_intermediary( | ||
resp.status, resp.reason, resp.headers, await resp.text() | ||
) from None | ||
except asyncio.TimeoutError as e: | ||
raise exceptions.TwirpServerException( | ||
code=errors.Errors.DeadlineExceeded, | ||
message=str(e) or "request timeout", | ||
meta={"original_exception": e}, | ||
) | ||
except aiohttp.ServerConnectionError as e: | ||
raise exceptions.TwirpServerException( | ||
code=errors.Errors.Unavailable, | ||
message=str(e), | ||
meta={"original_exception": e}, | ||
) |
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 |
---|---|---|
|
@@ -101,6 +101,6 @@ def _get_encoder_decoder(self, endpoint, headers): | |
else: | ||
raise exceptions.TwirpServerException( | ||
code=errors.Errors.BadRoute, | ||
message="unexpected Content-Type: " + ctype | ||
message="unexpected Content-Type: " + str(ctype) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
) | ||
return encoder, decoder |
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
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.
The go implementation doesn't create a client automatically.
I think it should be the same here.
It can be used like this:
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.
Great! Yes, I agree, and I didn't love the idea of creating the session automatically, but I wasn't sure if some would prefer a simpler way to use it without having to provide and manage the underlying session. (Because the existing sync implementation with
requests
has no such session requirement, I was hesitant to add one, and was trying to keep the usage similar for those who wanted to keep it that way.)However:
aiohttp.ClientSession
can only be created within a coroutine, so if you are required to provide a session when you init the twirp client then you can only do so within a coroutine. (This is not how we currently init our client dependencies within our app, so I had planned to pass the client session with each request, but that might feel cumbersome to some folks)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.
I think you can solve your problem by turning the client into an async context manager.
It would be something like that:
with_session
can be implemented like this:Or using a factory? (need testing)
You would need to implement
__aenter__
and__aexit__
onAsyncTwirpClient
to call the functions on the session.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.
After thinking a bit more about this, maybe you could just pass the
http_client_options
instead of a factory to the constructor when creating the twirp client.Then you can use
__aenter__
and__aexit__
to create a temporary session which can be reused for several calls.But it could also create a session directly inside
_make_request
on each call for simplicity when there is no need to reuse a client session.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.
Thanks for the suggestions @MiLk! I don't mind passing the session in each request, but do you think I should add
with_session
orsession_factory
as something that's generally useful for others? I pushed some changes removing the auto-created client, and would be fine merging as-is but would like to cover other common use cases.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 didn't see that the
session
could be passed directly with each request.You can disregard my previous suggestions.
I think that's enough, and there is no need to make the code more complex.