This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new 2be4b8aab0 [Cherry-pick to branch-1.3] [#13736] fix(python-client):
make close idempotent (#13741) (#13754)
2be4b8aab0 is described below
commit 2be4b8aab0853c9fd8400beae6df53b7a24d2078
Author: Thunderkeg <[email protected]>
AuthorDate: Sun Oct 11 07:36:57 2026 +0800
[Cherry-pick to branch-1.3] [#13736] fix(python-client): make close
idempotent (#13741) (#13754)
**Cherry-pick Information:**
- Source PR: #13741
- Target branch: `branch-1.3`
- Status: ✅ Clean cherry-pick (no conflicts)
Signed-off-by: jiangxt2 <[email protected]>
---
.../client-python/gravitino/utils/http_client.py | 7 ++-
.../unittests/auth/test_oauth2_token_provider.py | 23 +++++++++
.../tests/unittests/test_gravitino_client.py | 17 +++++++
.../tests/unittests/test_http_client_close.py | 58 ++++++++++++++++++++++
4 files changed, 104 insertions(+), 1 deletion(-)
diff --git a/clients/client-python/gravitino/utils/http_client.py
b/clients/client-python/gravitino/utils/http_client.py
index 0aaa0e3940..c7fece2bd5 100644
--- a/clients/client-python/gravitino/utils/http_client.py
+++ b/clients/client-python/gravitino/utils/http_client.py
@@ -97,6 +97,7 @@ class HTTPClient:
self.timeout = gravitino_client_config.get_client_request_timeout()
self.is_debug = is_debug
self.auth_data_provider = auth_data_provider
+ self._closed = False
def _build_url(self, endpoint=None, params=None):
url = self.host
@@ -244,10 +245,14 @@ class HTTPClient:
)
def close(self):
- self._request("close", "/")
+ if self._closed:
+ return
+
if self.auth_data_provider is not None:
self.auth_data_provider.close()
+ self._closed = True
+
def unpack(path: str):
def decorator(func):
diff --git
a/clients/client-python/tests/unittests/auth/test_oauth2_token_provider.py
b/clients/client-python/tests/unittests/auth/test_oauth2_token_provider.py
index d7c3928d05..ddf998c011 100644
--- a/clients/client-python/tests/unittests/auth/test_oauth2_token_provider.py
+++ b/clients/client-python/tests/unittests/auth/test_oauth2_token_provider.py
@@ -20,17 +20,40 @@ from unittest.mock import patch
from gravitino.auth.auth_constants import AuthConstants
from gravitino.auth.default_oauth2_token_provider import
DefaultOAuth2TokenProvider
+from gravitino.auth.oauth2_token_provider import OAuth2TokenProvider
from gravitino.exceptions.base import (
BadRequestException,
IllegalArgumentException,
UnauthorizedException,
)
+from gravitino.utils.http_client import HTTPClient
from tests.unittests.auth import mock_base
OAUTH_PORT = 1082
+class _TestOAuth2TokenProvider(OAuth2TokenProvider):
+ def _get_access_token(self):
+ return None
+
+ def validate(self):
+ pass
+
+
class TestOAuth2TokenProvider(unittest.TestCase):
+ @patch("gravitino.utils.http_client.build_opener")
+ def test_close_delegates_without_request(self, build_opener):
+ provider = _TestOAuth2TokenProvider("http://localhost";)
+
+ with patch.object(
+ HTTPClient, "close", autospec=True, side_effect=HTTPClient.close
+ ) as client_close:
+ provider.close()
+ provider.close()
+
+ self.assertEqual(2, client_close.call_count)
+ build_opener.assert_not_called()
+
def test_provider_init_exception(self):
with self.assertRaises(IllegalArgumentException):
_ = DefaultOAuth2TokenProvider(uri="test")
diff --git a/clients/client-python/tests/unittests/test_gravitino_client.py
b/clients/client-python/tests/unittests/test_gravitino_client.py
index 80ebda8c16..9b3aad87da 100644
--- a/clients/client-python/tests/unittests/test_gravitino_client.py
+++ b/clients/client-python/tests/unittests/test_gravitino_client.py
@@ -15,6 +15,7 @@
# specific language governing permissions and limitations
# under the License.
import unittest
+from unittest.mock import Mock, patch
from gravitino import GravitinoAdminClient, GravitinoClient
from gravitino.client.gravitino_client_config import GravitinoClientConfig
@@ -25,6 +26,22 @@ from tests.unittests import mock_base
@mock_base.mock_data
class TestMetalake(unittest.TestCase):
# pylint: disable=W0212
+ def test_gravitino_client_close_delegates_without_request(self,
*mock_methods):
+ auth_data_provider = Mock()
+ gravitino_client = GravitinoClient(
+ uri="http://localhost:8090";,
+ metalake_name="test",
+ check_version=False,
+ auth_data_provider=auth_data_provider,
+ )
+
+ with patch("gravitino.utils.http_client.build_opener") as build_opener:
+ gravitino_client.close()
+ gravitino_client.close()
+
+ auth_data_provider.close.assert_called_once_with()
+ build_opener.assert_not_called()
+
def test_gravitino_client_headers(self, *mock_methods):
expected_headers = {
"k1": "v1",
diff --git a/clients/client-python/tests/unittests/test_http_client_close.py
b/clients/client-python/tests/unittests/test_http_client_close.py
new file mode 100644
index 0000000000..9f1252409e
--- /dev/null
+++ b/clients/client-python/tests/unittests/test_http_client_close.py
@@ -0,0 +1,58 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import unittest
+from unittest.mock import Mock, patch
+
+from gravitino.utils.http_client import HTTPClient
+
+
+class TestHTTPClientClose(unittest.TestCase):
+ @patch("gravitino.utils.http_client.build_opener")
+ def test_close_without_auth_provider_is_idempotent(self, build_opener):
+ client = HTTPClient("http://localhost";)
+
+ client.close()
+ client.close()
+
+ build_opener.assert_not_called()
+
+ @patch("gravitino.utils.http_client.build_opener")
+ def test_close_closes_auth_provider_once(self, build_opener):
+ auth_data_provider = Mock()
+ client = HTTPClient("http://localhost";,
auth_data_provider=auth_data_provider)
+
+ client.close()
+ client.close()
+
+ auth_data_provider.close.assert_called_once_with()
+ build_opener.assert_not_called()
+
+ @patch("gravitino.utils.http_client.build_opener")
+ def test_close_retries_after_auth_provider_failure(self, build_opener):
+ auth_data_provider = Mock()
+ auth_data_provider.close.side_effect = [RuntimeError("cleanup
failed"), None]
+ client = HTTPClient("http://localhost";,
auth_data_provider=auth_data_provider)
+
+ with self.assertRaisesRegex(RuntimeError, "cleanup failed"):
+ client.close()
+
+ client.close()
+ client.close()
+
+ self.assertEqual(2, auth_data_provider.close.call_count)
+ build_opener.assert_not_called()
