Repository: googleapis/python-cloud-core

Issue:
Clients do not clean up Requests sessions
#### Environment details

  - OS type and version: macOS
  - Python version: 3.9
  - `google-cloud-core` version: 1.4.3
  - `google-cloud-storage` version: 1.33.0 (since it's related)

#### Steps to reproduce

This is a sibling issue to https://github.com/googleapis/google-auth-library-python/issues/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](https://docs.pytest.org/en/stable/usage.html#warning-about-unraisable-exceptions-and-unhandled-thread-exceptions) 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](https://github.com/googleapis/python-storage) with e.g.

```python
        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](https://github.com/googleapis/python-cloud-core/blob/d01c825d4b7638e76d9d2883baba5a91c884e310/google/cloud/client.py#L172-L184) is run, creating a new `requests.Session()` that will be _eventually_ GC'd but is otherwise not [`close()`d](https://requests.readthedocs.io/en/latest/api/#requests.Session.close), 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()`.

```python
        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 `Client`s; by default it should probably only be something like
  ```python
  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, `Client`s could be used with [`contextlib.closing()`](https://docs.python.org/3.9/library/contextlib.html#contextlib.closing). Additionally, one could add a minimal context management protocol implementation á la

```python
            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).

Discussion:

Patch:
diff --git a/google/cloud/client.py b/google/cloud/client.py
index 0697741..9e09450 100644
--- a/google/cloud/client.py
+++ b/google/cloud/client.py
@@ -214,6 +214,21 @@ class Client(_ClientFactoryMixin):
             self._http_internal.configure_mtls_channel(self._client_cert_source)
         return self._http_internal
 
+    def close(self):
+        """Clean up transport, if set.
+
+        Suggested use:
+
+        .. code-block:: python
+
+           import contextlib
+
+           with contextlib.closing(client):  # closes on exit
+               do_something_with(client)
+        """
+        if self._http_internal is not None:
+            self._http_internal.close()
+
 
 class _ClientProjectMixin(object):
     """Mixin to allow setting the project on the client.


Test patch:
diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py
index 9eeb757..1b933a2 100644
--- a/tests/unit/test_client.py
+++ b/tests/unit/test_client.py
@@ -205,6 +205,21 @@ class TestClient(unittest.TestCase):
         file_open.assert_called_once_with(mock.sentinel.filename, "r", encoding="utf-8")
         constructor.assert_called_once_with(info)
 
+    def test_close_w__http_internal_none(self):
+        credentials = _make_credentials()
+        client_obj = self._make_one(credentials=credentials, _http=None)
+
+        client_obj.close()  # noraise
+
+    def test_close_w__http_internal_set(self):
+        credentials = _make_credentials()
+        http = mock.Mock(spec=["close"])
+        client_obj = self._make_one(credentials=credentials, _http=http)
+
+        client_obj.close()
+
+        http.close.assert_called_once_with()
+
 
 class Test_ClientProjectMixin(unittest.TestCase):
     @staticmethod