This is an automated email from the ASF dual-hosted git repository.
shahar1 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new fe8e5dfef07 Close S3 download_file handle once the download finishes
or fails (#69084)
fe8e5dfef07 is described below
commit fe8e5dfef077c57a5df271bc4c090300273994c1
Author: itej13 <[email protected]>
AuthorDate: Mon Jun 29 01:48:36 2026 +0530
Close S3 download_file handle once the download finishes or fails (#69084)
S3Hook.download_file opened a local file (or NamedTemporaryFile with
delete=False), streamed the object into it, and returned the path
without ever closing the handle, relying on garbage collection to
release the descriptor. On a download error the handle leaked the same
way. Closing it in a finally releases the descriptor on every path while
keeping the downloaded file on disk for the caller.
---
.../src/airflow/providers/amazon/aws/hooks/s3.py | 18 ++++++++++++------
.../amazon/tests/unit/amazon/aws/hooks/test_s3.py | 15 +++++++++++++++
2 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/providers/amazon/src/airflow/providers/amazon/aws/hooks/s3.py
b/providers/amazon/src/airflow/providers/amazon/aws/hooks/s3.py
index 616506e2134..626ac19730d 100644
--- a/providers/amazon/src/airflow/providers/amazon/aws/hooks/s3.py
+++ b/providers/amazon/src/airflow/providers/amazon/aws/hooks/s3.py
@@ -1629,12 +1629,18 @@ class S3Hook(AwsBaseHook):
extra_args = {**self.extra_args}
if self._requester_pays:
extra_args["RequestPayer"] = "requester"
- s3_obj.download_fileobj(
- file,
- ExtraArgs=extra_args,
- Config=self.transfer_config,
- )
- file.flush()
+ try:
+ s3_obj.download_fileobj(
+ file,
+ ExtraArgs=extra_args,
+ Config=self.transfer_config,
+ )
+ file.flush()
+ finally:
+ # The file is created with delete=False / opened by us, so closing
the
+ # handle keeps the data on disk while releasing the descriptor
even when
+ # the download raises.
+ file.close()
get_hook_lineage_collector().add_input_asset(
context=self, scheme="s3", asset_kwargs={"bucket": bucket_name,
"key": key}
)
diff --git a/providers/amazon/tests/unit/amazon/aws/hooks/test_s3.py
b/providers/amazon/tests/unit/amazon/aws/hooks/test_s3.py
index 8b2d70c0c09..c4cc20d2231 100644
--- a/providers/amazon/tests/unit/amazon/aws/hooks/test_s3.py
+++ b/providers/amazon/tests/unit/amazon/aws/hooks/test_s3.py
@@ -1530,6 +1530,21 @@ class TestAwsS3Hook:
assert mock_file.name == output_file
+ @mock.patch("airflow.providers.amazon.aws.hooks.s3.NamedTemporaryFile")
+ def test_download_file_closes_handle_on_error(self, mock_temp_file,
tmp_path):
+ mock_file = mock_temp_file.return_value
+ mock_file.name = str(tmp_path / "airflow_tmp_test_s3_hook")
+ s3_hook = S3Hook(aws_conn_id="s3_test")
+ s3_hook.check_for_key = Mock(return_value=True)
+ s3_obj = Mock()
+ s3_obj.download_fileobj = Mock(side_effect=OSError("download failed"))
+ s3_hook.get_key = Mock(return_value=s3_obj)
+
+ with pytest.raises(OSError, match="download failed"):
+ s3_hook.download_file(key="test_key", bucket_name="test_bucket")
+
+ mock_file.close.assert_called_once_with()
+
@mock.patch("airflow.providers.amazon.aws.hooks.s3.NamedTemporaryFile")
def test_download_file_exposes_lineage(self, mock_temp_file, tmp_path,
hook_lineage_collector):
mock_file = mock_temp_file.return_value
