-
-
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
Too many "async with" #5385
Comments
I think one problem with your proposed solution is that it would become less clear when you do need to close it, such as with: async with resp:
assert resp.status == 200 I think it could be reasonable to have a shortcut with an interface something like one of these: await session.get(url, text=True)
await session.get_text(url)
await session.get_read(type="text") With the return type either being the string returned from |
I'm not sure what exactly the issue is. The solution, which is already possible with aiohttp, e.g.: async def main():
async with aiohttp.ClientSession() as session:
resposne = await session.get("http://httpbin.org")
print(await resposne.text()) is what you're saying is an issue? since it involves using Regardless, you're not forced to use async def main():
session = aiohttp.ClientSession()
resposne = await session.get("http://httpbin.org")
print(await resposne.text())
await session.close() As for your alternative, as the documentation says:
The connection should already be released when the payload is fully read or when an error is encountered, so this would be redundant. For code block usage, see https://docs.github.com/en/free-pro-team@latest/github/writing-on-github/creating-and-highlighting-code-blocks. |
That code looks wrong to me (the response is never closed explicitly). Compare https://docs.aiohttp.org/en/stable/client_quickstart.html#make-a-request This issue is the inner async with: async def main():
async with aiohttp.ClientSession() as session:
async with await session.get("http://httpbin.org") as response:
print(await response.text()) |
In fact I just ran your example, and it never printed or finished. I had to Ctrl+C. |
It's working fine here |
@Harmon758 |
@Dreamsorcerer |
It will work if you indent the last line into the with block (which I see you changed now). But, then the response object is never explicitly closed, which is probably not what you want. So, you should then have a This is the equivalent of doing with open(...) as f:
f.read() |
@Dreamsorcerer |
And the shortcut available for that |
@flyinghu123 the response object does not hold the HTTP response body to give end-users more control. Imagine if the response is large, then it'd easy to consume gigabytes of memory. You'll get |
Hmm, I think it might be the request to httpbin.org hanging.
Does the response need to be explicitly closed? I would have added it at the end, but I checked |
@webknjaz |
@webknjaz But, the e.g. |
@Dreamsorcerer |
Ah, interesting. I just assumed it would |
It seems so. |
I agree that it's better to explicitly close, but in this case, I was trying to mimic the proposed solution in the issue as closely as possible. I unindented the last line because the indentation was unclear in the initial proposed solution and it seemed from the comment that it was meant to be outside the |
This issue reminds me about #4346. I think every lazy developer will be pleased if we finally implement it :) |
As one more example of shortcuts like this, sqlalchemy has:
And there is a shortcut for that: |
Just some additional comments on this. I believe that any time I would reject the very original suggestion though, to do something like httpx. httpx has the same |
🐣 Is your feature request related to a problem? Please describe.
I'm always frustrated when I use get or post method.
I must use "async with" to use get or post method.
So there's a lot of "async with" nesting.
💡 Describe the solution you'd like
I hope that I can use it just like httpx.
Ex:
❓ Describe alternatives you've considered
Add "self.release" after "await self.read()".
Because I don't need connection after this.
📋 Additional context
The text was updated successfully, but these errors were encountered: