Merge pull request '[patch] Fix MinIO get_object response leak in MinioAdapter.get()' (#32) from fix/minio-get-object-response-cleanup into main
Release on merge to main / release (push) Successful in 14s
Test Python Package / unit-tests (push) Successful in 15s
Code Quality Pipeline / code-quality (push) Successful in 47s
Test Python Package / integration-tests (push) Successful in 54s
Test Python Package / coverage-report (push) Successful in 15s
Release on merge to main / release (push) Successful in 14s
Test Python Package / unit-tests (push) Successful in 15s
Code Quality Pipeline / code-quality (push) Successful in 47s
Test Python Package / integration-tests (push) Successful in 54s
Test Python Package / coverage-report (push) Successful in 15s
Reviewed-on: https://gitea.lille-vemmelund.dk/brian/python-repositories/pulls/32
This commit was merged in pull request #32.
This commit is contained in:
@@ -16,8 +16,14 @@ jobs:
|
|||||||
|
|
||||||
- name: Check PR title when source files change
|
- name: Check PR title when source files change
|
||||||
env:
|
env:
|
||||||
PR_TITLE: ${{ github.event.pull_request.title }}
|
API_URL: ${{ vars.API_URL }}
|
||||||
|
REPO_OWNER: ${{ github.repository_owner }}
|
||||||
|
REPO_NAME: ${{ github.event.repository.name }}
|
||||||
|
PR_NUMBER: ${{ github.event.pull_request.number }}
|
||||||
|
CI_RUNNER_TOKEN: ${{ secrets.CI_RUNNER_TOKEN }}
|
||||||
run: |
|
run: |
|
||||||
git fetch origin "${{ github.base_ref }}"
|
git fetch origin "${{ github.base_ref }}"
|
||||||
|
PR_TITLE=$(scripts/ci/fetch-pr-title.sh)
|
||||||
|
echo "Live PR title: ${PR_TITLE}"
|
||||||
git diff --name-only "origin/${{ github.base_ref }}...HEAD" \
|
git diff --name-only "origin/${{ github.base_ref }}...HEAD" \
|
||||||
| scripts/ci/check-pr-title.sh "$PR_TITLE"
|
| scripts/ci/check-pr-title.sh "$PR_TITLE"
|
||||||
|
|||||||
@@ -152,6 +152,7 @@ class MinioAdapter(ObjectRepositoryInterface, ConnectionAwareAdapter):
|
|||||||
assert self._client is not None and self._bucket_name is not None
|
assert self._client is not None and self._bucket_name is not None
|
||||||
# Get data from bucket
|
# Get data from bucket
|
||||||
# N.B. bucket name is set when connecting
|
# N.B. bucket name is set when connecting
|
||||||
|
response = None
|
||||||
try:
|
try:
|
||||||
response = self._client.get_object(
|
response = self._client.get_object(
|
||||||
bucket_name=self._bucket_name,
|
bucket_name=self._bucket_name,
|
||||||
@@ -175,6 +176,10 @@ class MinioAdapter(ObjectRepositoryInterface, ConnectionAwareAdapter):
|
|||||||
self.logger.error(repr(exc))
|
self.logger.error(repr(exc))
|
||||||
except Exception as exc: # pylint: disable=broad-except
|
except Exception as exc: # pylint: disable=broad-except
|
||||||
self.logger.error(repr(exc))
|
self.logger.error(repr(exc))
|
||||||
|
finally:
|
||||||
|
if response is not None:
|
||||||
|
response.close()
|
||||||
|
response.release_conn()
|
||||||
return None
|
return None
|
||||||
|
|
||||||
def delete(self, object_name: str) -> None:
|
def delete(self, object_name: str) -> None:
|
||||||
|
|||||||
Executable
+16
@@ -0,0 +1,16 @@
|
|||||||
|
#!/usr/bin/env bash
|
||||||
|
set -euo pipefail
|
||||||
|
|
||||||
|
# Fetch the current PR title from the Gitea API.
|
||||||
|
# Requires: API_URL, REPO_OWNER, REPO_NAME, PR_NUMBER, CI_RUNNER_TOKEN
|
||||||
|
|
||||||
|
: "${API_URL:?API_URL is required}"
|
||||||
|
: "${REPO_OWNER:?REPO_OWNER is required}"
|
||||||
|
: "${REPO_NAME:?REPO_NAME is required}"
|
||||||
|
: "${PR_NUMBER:?PR_NUMBER is required}"
|
||||||
|
: "${CI_RUNNER_TOKEN:?CI_RUNNER_TOKEN is required}"
|
||||||
|
|
||||||
|
curl -sf \
|
||||||
|
"${API_URL}/repos/${REPO_OWNER}/${REPO_NAME}/pulls/${PR_NUMBER}" \
|
||||||
|
-H "Authorization: token ${CI_RUNNER_TOKEN}" \
|
||||||
|
| python3 -c 'import json, sys; print(json.load(sys.stdin)["title"])'
|
||||||
@@ -91,3 +91,36 @@ def test_connect_disconnects_before_reconnect(
|
|||||||
|
|
||||||
assert adapter._client is new_client
|
assert adapter._client is new_client
|
||||||
new_client.list_buckets.assert_called_once()
|
new_client.list_buckets.assert_called_once()
|
||||||
|
|
||||||
|
|
||||||
|
def test_get_closes_response_on_success() -> None:
|
||||||
|
"""get() must close and release the get_object HTTP response."""
|
||||||
|
mock_client = MagicMock(spec=Minio)
|
||||||
|
mock_client.bucket_exists.return_value = True
|
||||||
|
mock_response = MagicMock()
|
||||||
|
mock_response.read.side_effect = [b"data", b""]
|
||||||
|
mock_client.get_object.return_value = mock_response
|
||||||
|
adapter = MinioAdapter(config=TEST_MINIO_CONFIG, client=mock_client)
|
||||||
|
|
||||||
|
result = adapter.get("some-object")
|
||||||
|
|
||||||
|
assert result is not None
|
||||||
|
assert result.read() == b"data"
|
||||||
|
mock_response.close.assert_called_once()
|
||||||
|
mock_response.release_conn.assert_called_once()
|
||||||
|
|
||||||
|
|
||||||
|
def test_get_closes_response_when_read_fails() -> None:
|
||||||
|
"""get() must close and release the response even if read() raises."""
|
||||||
|
mock_client = MagicMock(spec=Minio)
|
||||||
|
mock_client.bucket_exists.return_value = True
|
||||||
|
mock_response = MagicMock()
|
||||||
|
mock_response.read.side_effect = OSError("connection reset")
|
||||||
|
mock_client.get_object.return_value = mock_response
|
||||||
|
adapter = MinioAdapter(config=TEST_MINIO_CONFIG, client=mock_client)
|
||||||
|
|
||||||
|
result = adapter.get("some-object")
|
||||||
|
|
||||||
|
assert result is None
|
||||||
|
mock_response.close.assert_called_once()
|
||||||
|
mock_response.release_conn.assert_called_once()
|
||||||
|
|||||||
Reference in New Issue
Block a user