From 1a011c52ff555443e66421c2574dc04d3b7df468 Mon Sep 17 00:00:00 2001 From: Chris Wesseling Date: Fri, 15 Sep 2023 17:20:57 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B[#705]=20Properly=20close=20'file:/?= =?UTF-8?q?/'=20resources?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I bumped into this when opening several wsdl which in turn opened lots of xsd, with 'file://' scheme. The issue was that the `resp.raw.close` nor `resp.raw.release_conn` set in the `FileAdapter` were ever called. It's unclear to me whether this should be fixed in requests. It doesn't do that great a job at resource management for the naive user aka Human™. It makes sense to me that exhaustively reading `Response.raw` should close it unless the caller explicitly set `stream` on the request. Probably by using this `closing` pattern in the generator in `Response.iter_content`. Workarounds without this fix: - using a scheme-less url as zeep will assume it's a local path and open the file as a context manager. - use one of the caches from `zeep.cache` to hide duplicate open resources. --- src/zeep/transports.py | 8 ++++---- tests/test_transports.py | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/zeep/transports.py b/src/zeep/transports.py index 8e6970d2..b5bc00d6 100644 --- a/src/zeep/transports.py +++ b/src/zeep/transports.py @@ -1,6 +1,6 @@ import logging import os -from contextlib import contextmanager +from contextlib import contextmanager, closing from urllib.parse import urlparse import requests @@ -114,7 +114,6 @@ def load(self, url): scheme = urlparse(url).scheme if scheme in ("http", "https", "file"): - if self.cache: response = self.cache.get(url) if response: @@ -133,8 +132,9 @@ def load(self, url): def _load_remote_data(self, url): self.logger.debug("Loading remote data from: %s", url) response = self.session.get(url, timeout=self.load_timeout) - response.raise_for_status() - return response.content + with closing(response): + response.raise_for_status() + return response.content @contextmanager def settings(self, timeout=None): diff --git a/tests/test_transports.py b/tests/test_transports.py index 2fc87013..ce8ad193 100644 --- a/tests/test_transports.py +++ b/tests/test_transports.py @@ -49,6 +49,7 @@ def test_load_file_unix(): result = transport.load("file:///usr/local/bin/example.wsdl") assert result == b"x" m_open.assert_called_once_with("/usr/local/bin/example.wsdl", "rb") + m_open.return_value.close.assert_called() @pytest.mark.skipif(os.name != "nt", reason="test valid for windows platform only")