This repository has been archived by the owner on Apr 14, 2022. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 22
Add API for Request, SyncRequestData, and AsyncRequestData #151
Closed
Closed
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
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 +1,2 @@ | ||
from .models import Request, SyncRequestData, AsyncRequestData | ||
from .status_codes import StatusCode |
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,81 @@ | ||
import typing | ||
|
||
HeadersType = typing.Union[ | ||
typing.Mapping[str, str], | ||
typing.Mapping[bytes, bytes], | ||
typing.Iterable[typing.Tuple[str, str]], | ||
typing.Iterable[typing.Tuple[bytes, bytes]], | ||
] | ||
|
||
class SyncRequestData: | ||
"""Represents a synchronous request data object. Basically a wrapper around whatever | ||
a user passes in via 'data', 'files', 'json', etc parameters. We can take bytes / | ||
strings, iterators of bytes or strings, and files. We can also sub-class / implement | ||
a file-like interface for things like multipart-formdata so it's possible to | ||
rewind (unlike the current urllib3 interface). | ||
|
||
Maybe in the future we can expose details about when an object can be sent via | ||
'socket.sendfile()', etc. This would have to be communicated somehow to the | ||
low-level backend streams. | ||
|
||
When we're handed a file-like object we should take down the starting point | ||
in the file via .tell() so we can rewind and put the pointer back after | ||
the seek() call in '.content_length' | ||
|
||
Ref: https://github.com/python-trio/urllib3/issues/135 | ||
""" | ||
|
||
def read(self, nbytes: int) -> bytes: | ||
"""Don't know what to call this? Grab some data from the pool of data | ||
to be sent across the wire. | ||
""" | ||
@property | ||
def content_length(self) -> typing.Optional[int]: | ||
"""We can get a proper content-length for bytes, strings, file-like objects | ||
that are opened in binary mode (is that detectable somehow?). If we hand | ||
back 'None' from this property it means that the request should use | ||
'Transfer-Encoding: chunked' | ||
""" | ||
def rewind(self) -> None: | ||
"""This function rewinds the request data so that it can be retransmitted | ||
in the case of a timeout/socket error on an idempotent request, a redirect, | ||
etc so that the new request can be sent. This works for file-like objects | ||
and bytes / strings (where it's a no-op). | ||
""" | ||
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. Should also add a |
||
@property | ||
def is_rewindable(self) -> bool: | ||
"""We should return a bool whether .rewind() will explode or not.""" | ||
|
||
class AsyncRequestData(SyncRequestData): | ||
"""The same as above except also accepts async files (we'll have to | ||
handle all the possible APIs here?) and async iterators. | ||
""" | ||
|
||
async def read(self, nbytes: int) -> bytes: ... | ||
async def rewind(self) -> None: ... | ||
|
||
class Request: | ||
"""Requests aren't painted async or sync, only their data is. | ||
By the time the request has been sent on the network and we'll | ||
get a response back the request will be attached to the response | ||
via 'SyncResponse.request'. At that point we can remove the 'data' | ||
parameter from the Request and only have the metadata left so | ||
users can't muck around with a spent Request body. | ||
|
||
The 'url' type now is just a string but will be a full-featured | ||
type in the future. Requests has 'Request.url' as a string but | ||
we'll want to expose the whole URL object to do things like | ||
'request.url.origin' downstream. | ||
|
||
Also no reason to store HTTP version here as the final version | ||
of the request will be determined after the connection has | ||
been established. | ||
""" | ||
|
||
def __init__( | ||
self, | ||
method: str, | ||
url: str, | ||
headers: HeadersType, | ||
data: typing.Union[SyncRequestData, AsyncRequestData]=None, | ||
): ... |
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.
Regarding there only being one interface, I think we should have something like
SyncRequestData.sync_read()
andAsyncRequestData.async_read()
or maybe we switch to usingasync for chunk in request.data
which would automatically get transformed by unasync but we'd forfeit the ability to grab a certain amount of data. I guess that interface is easier to implement anyways?The reason I think this would be good to have the split is you'd only have to subclass
AsyncRequestData
and could technically use that class in both async and synchronous requests. Would just need to have good error reporting if you're passed input data that is for example, an async generator, and then using it in a sync session. I'm guessing that wouldn't happen often and an error message would fix any ambiguity.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.
Here's an example of my plan in action:
Suddenly this one class can be used for both synchronous and asynchronous requests. We keep the
is_rewindable
(or maybe rename it tois_replayable
? or something else) and then we'd signal a "rewind" by calling__aiter__
or__iter__
again.