python-ecosys/urequests: add streaming API support and cache standard responses#426
python-ecosys/urequests: add streaming API support and cache standard responses#426mkomon wants to merge 0 commit into
Conversation
|
By the way I am using urequests with these two commits with lichess.org API and all works well, both standard and streaming API requests. Without this patch streaming requests don't work at all as they read only the immediate response and close the socket immediately. |
| while chunk: | ||
| data += chunk | ||
| chunk = self.raw.read() | ||
| return data |
There was a problem hiding this comment.
Does it make sense to combine all data here? Why not just return the result of the first read? The caller must anyway keep calling this function until it has all data.
There was a problem hiding this comment.
Good point, that makes sense and the loop just adds an unnecessary read. I will test it with return self.raw.read() and report back.
| if stream: | ||
| resp.raw.setblocking(False) | ||
| else: | ||
| resp.content |
There was a problem hiding this comment.
I'm not sure it makes sense to pre-read the content here. Some uses of this may not need the content and then it shouldn't waste time and memory retrieving it.
There was a problem hiding this comment.
I have not considered such use case but it makes total sense. There is no need to read the response like the original requests package does. I'll remove 9b5d30c. (removed with a force-push since it was the last commit)
2 participants
As the title says.