cachecontrol
cachecontrol copied to clipboard
fix streaming error "Unable to determine whether fp is closed."
trying to fix bug with new SeparateBodyBaseCache,
hit this when I write a custom SQLite cache backend with streaming resp.
- urllib3/util/response.py
def is_fp_closed(obj):
"""
Checks whether a given file-like object is closed.
:param obj:
The file-like object to check.
"""
try:
# Check `isclosed()` first, in case Python3 doesn't set `closed`.
# GH Issue #928
return obj.isclosed()
except AttributeError:
pass
try:
# Check via the official file-like-object way.
return obj.closed
except AttributeError:
pass
try:
# Check if the object is a container for another file-like object that
# gets released on exhaustion (e.g. HTTPResponse).
return obj.fp is None
except AttributeError:
pass
raise ValueError("Unable to determine whether fp is closed.")
The comment above with code, and the pull request, don't really seem related, and it's also not clear what the bug is. So you can explain:
- What is the bug is that motivated the PR?
- What that comment is for?
@itamarst I haven't finished the full trace what path involve the issue, I just dig out which part raise the exception,
as I wrote a custom sqlite backend which use SeparateBodyBaseCache
as base class I return the body part as bytes,
the streaming path require the body have file-like interface in order to properly function,
the patch just a quick fix that wrap the body part into file-like obj if it's not one.
- If you're returning a giant pile of bytes in one go, that doesn't give you the advantage of
SeparateBodyBaseCache
; you're still using a lot of memory. So I'm not sure you want to bother with that base class unless you're actually streaming? - If you do want to use it like this anyway, wouldn't wrapping the bytes with
BytesIO
in your class make more sense, rather than patching CacheControl? The interface uses a file-like object for a reason.
@itamarst I am not really working on a giant pile of bytes, just some MPEG-DASH,
this patch is a simple version to catch any incorrect implementation, we need a better document about what the interface should return or write better error handling to remind developer, not a ValueError
exception hidden in another lib without any hint.
the original implement always cause a full write on HTTP 304 Not Modified
, separate the header and body write reduce this side effect significantly, this is why I work on this.