-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Alternative simplified HTTP client API #4346
Comments
A few things. Since it is for simplified usage please consider If we pin the
and
Since Websockets are not supported, at least for the first version -- I agree. I suggest starting from the very minimalistic API, extend the list of supported features incrementally. Our intention should be not
Regarding testing: please don't use mocking at all, it is not required for the discussed API. Take |
Got it. My preference usually goes to something close to the domain, though. So I'd suggest "requests" or "http":
Or:
But honestly I can work with any name, just chose the one you like.
I was thinking that ".text" could lazily read and close the connection, as to mitigate the perf cost we already pays. But in retrospect, this will cost more memory and make the code more complex and harder to test. attrs class it is.
Alright, but I need to store the bytes somewhere: any request to an image, a zip or an excel file is a legit case for the simplified API, but we won't decode that as text. Also, I expect json() will run faster if it is fed from bytes, not str. Given I'll have it anyway, can I just publicly expose it, as a "payload" or a "bytes" attribute ?
Yes.
Ok. I'll make it a functional API a the root of the module. No need for a class.
Thank you. |
Let's wait for a while, maybe the best name will come up.
Let's use |
I do like
But the top-level namespace is already But I have a soft spot for more cutesy names like |
|
Cutesie names always sound cool to the authors, but if you open and stumble uppon an unknow code base with it, it gives zero information of what it does. It's the same problem with "nurseries" in trio and why there is an entire thread still open about it : python-trio/trio#504"simple" is less ambiguous, in fact, it removes ambiguity by anwsering by itself what's so special about this new API. This is good.15:46, 16 novembre 2019, Martijn Pieters <notifications@github.com>:I do like nutshell; async HTTP requests in a nutshell. It neatly conveys that this is the digest reader version of the full API. The downside is that those building a server may ask for a nutshell simplification of the server APIs too.
My preference usually goes to something close to the domain, though. So I'd suggest "requests" or "http":
But the top-level namespace is already aiohttp. aiohttp.http feels rather redundant, aiohttp.requests both feels like either a copy of the requests project and at the same time not clearly defining why it is different from aiohttp.client (most developers don't know nor care that requests is a friendly wrapper around urllib3). If you want a literal name, go for aiohttp.simple.
But I have a soft spot for more cutesy names like nutshell.
—You are receiving this because you authored the thread.Reply to this email directly, view it on GitHub, or unsubscribe.
-- Sent from Yandex.Mail for mobile
|
Cutesie names have given us Flask and Click and other household names. But if the name |
Aren't fork or socket little playful names for such serious things? From my understanding, simple is mostly used as an adjective. Also, I recall very many books about programming that are called in a nutshell. Anyway, the name can be changed easily, it is not a show-stopper for starting simplified API. The thing is very useful and important regardless of the naming. |
Just to link these issues, I think the new proposed API above is a bad idea, but maybe some shortcut methods for simple cases could be included (e.g. |
Let's close this one in favour of #5385. |
As discussed with @asvetlov, here is a proposal for an additional API to make HTTP requests, in two parts:
aiohttp.client.HttpClientSession
: an API with reasonable performances, and simpler semantics thanaiohttp.ClientSession
. This is what most small to average programs will want to use. The most important feature being the preloading of the request body, but it doesn't support WebSocket.aiohttp.client.requests
: an API with weak performances, but very easy to play with, that will remind the hello world from therequests
library. This is what we can put in the README to ease people into the transition into the async world. It's also very nice to use in a shell or quick and dirty scripts.The existing
ClientSession
API is not going to be deprecated, just presented in the documentation as a more advanced use case for when you want to get the best performances and control possible. The two other API in fact usesClientSession
under the hood.The README and the documentation will reflect this, and will clearly mention the pros and cons of each API, with links to the other. In order to avoid overwhelming the new comer, and to also avoid confusion ("why 3 ways of doing the same thing ?"), we will not present them at the same time.
It should be progressive:
1 - README:
Demonstrates
aiohttp.client.requests
as a hello world, then leads with a phrase like:"
Learn more about those simple requests<doc>
. Want better performances than this ?Read about about the more efficent alternatives<doc>
."2 - The page "Learn more about those simple requests" leads to the
requests
API tutorial. At the end of this tutorial, a mention is made about the pros and cons of this API, and a link to theaiohttp.client.HttpClientSession
API tutorial is offered.3 - At the end of the
aiohttp.client.HttpClientSession
API tutorial, a mention is made about the pros and cons of this API, and a link to theaiohttp.ClientSession
API tutorial is offered.4 -
Read about about the more efficent alternatives<doc>
_. explains the pros and cons of each API, and link to both theaiohttp.client.HttpClientSession
and theaiohttp.ClientSession
API tutorial. It will recommand to give a try toaiohttp.client.HttpClientSession
, and to chooseaiohttp.ClientSession
only if performances, control or WebSocket are important to the user.API design
Look and feel
Current
ClientSession
APINew
aiohttp.client.requests
APINew
aiohttp.client.HTTPClientSession
APIDifferences with the
ClientSession
APIHTTPClientSession
HTTPClientSession
is similar toClientSession
, except:connector_owner
param: sharing a connector is an advance use case, leave it to the advanced API._responses_history
attribute: this will hold a reference to all created responses and will be looped over for cleaning onHTTPClientSession.close()
.ws_response_class
param:HTTPClientSession
targets HTTP only. For websocket, use the advanced API.ws_connect()
raisesClientTypeError("HTTPClientSession does not support WebSocket. Use ClientSession instead.")
as defined by:HTTPClientSession.request/get/post/... and requests.get/post/...
Same as ClientSession.request/get/post..., except:
chunked
param: body will be preloaded, so it doesn't make sense.content_length_limit
param: if content length is set too high, read only up to this limit as a safety mechanism. This is necesarry sinceread()
is automatically called. Do you think I should set it to None by default or just a very high value ? Should it raise an exception is limit is reached ?read_until_eof
param: always read up to the "Content-Length" header orcontent_length_limit
PreloadedClientResponse
object instead of aClientResponse
.ClientResponse
.ClientResponse.read()
immediatly called onPreloadedClientResponse
prior return.HTTPClientSession.content
is not aaiohttp.StreamReader
butbytes
.PreloadedClientResponse.read()
raisesResponseTypeError("PreloadedClientResponse content is automatically preloaded, don't call read(), access the 'content' and 'text' attributes.")
as defined by:PreloadedClientResponse.text
is not a coroutine, but a@property
that lazily decodes content on first read, then is replaced by a regular attribute containing the result for subsequent reads. Do you have utils to do that in the code base ?PreloadedClientResponse.encoding
attribute, preloaded at__init__
after.read()
. Can be set manually by the user to compensate for the facttext
is not a regular function anymore and you can't pass it params.encoding_errors
param: accept "errors", "replace", etc., stored inencoding_errors
attribute. Compensate for the facttext
is not a regular function anymore and you can't pass param. (This could be a good addition to ClientSession as well, in case one wants the same strat for all requests instead of passing it to each call)HTTPClientSession
inherits fromClientSession
.requests
will in fact be a singleton (just instanciated at the root of the module, no fancy__new__
tricks), and an instance ofBasicHTTPClient
.BasicHTTPClient
will not inherit from ClientSession, but will use composition to delegate the work toHTTPClientSession
and only expose .get/post/etcTesting
I'll scan test_client_response.py and test_client_session.py for inspiration (espacially for the mocking), but I don't intend to duplicate the tests in there.
For
HTTPClientSession
, I'll create:And only tests the API differences with their parent, plus the signature, the behavior of overriden methods and ensure closing of the response objects.
For
requests
, I'll create test_requests.py and test the entire high level API, as well as use mocks to ensure we are calling the cleaning code correctly.The text was updated successfully, but these errors were encountered: