Clients do not clean up Requests sessions #64
Description
Environment details
- OS type and version: macOS
- Python version: 3.9
google-cloud-coreversion: 1.4.3google-cloud-storageversion: 1.33.0 (since it's related)
Steps to reproduce
This is a sibling issue to googleapis/google-auth-library-python#658 and as such the below is similar to that issue.
Py.test 6.2 enables hard errors on unhandled thread exceptions and resource errors such as leaking sockets/SSL sockets/FDs/.... I've spent the last couple hours debugging various
ResourceWarning: unclosed <ssl.SSLSocket fd=12, family=AddressFamily.AF_INET, type=SocketKind.SOCK_STREAM, proto=0, laddr=('192.168.1.23', 60522), raddr=('216.58.207.202', 443)>
When initiating (e.g.) a google-cloud-storage storage client with e.g.
creds = self.get_client_credentials()
return storage.Client(project=creds.project_id, credentials=creds)the client object is initialized with no explicit _http object, and the default code path is run, creating a new requests.Session() that will be eventually GC'd but is otherwise not close()d, leading to the above warning (and a dropped connection and socket, which may have implications down the line).
Since google.core.client.Client doesn't implement e.g. the context manager protocol or close(), there's no officially sanctioned way to clean up that session; for a storage Client, I subclassed one as a workaround but it would be nice if Clients had an official close().
class CloseableStorageClient(storage.Client):
def __enter__(self):
return self
def __exit__(self, *args):
self._http.close()Resolution suggestion
At the very least, add close() to Clients; by default it should probably only be something like
def close(self):
if self._http_internal:
self._http_internal.close(), the semantics being a best-effort cleanup of resources allocated by the client, but not irreversible finalization.
With this, Clients could be used with contextlib.closing(). Additionally, one could add a minimal context management protocol implementation á la
def __enter__(self):
return self
def __exit__(self, *args):
self.close()If you folks think this is a good way to go, I can certainly write the PR :)
cc @busunkim96 (for having triaged the other issue).