From 9da5c50167ec8572ca3e4e9fd03d3f34dad163d0 Mon Sep 17 00:00:00 2001 From: odlmarce Date: Wed, 20 Dec 2023 21:40:45 +0100 Subject: [PATCH 01/10] set aiohttp.ClientSession to None after close() Signed-off-by: odlmarce --- opensearchpy/_async/http_aiohttp.py | 1 + opensearchpy/connection/http_async.py | 1 + 2 files changed, 2 insertions(+) diff --git a/opensearchpy/_async/http_aiohttp.py b/opensearchpy/_async/http_aiohttp.py index b1baf148..c49fd574 100644 --- a/opensearchpy/_async/http_aiohttp.py +++ b/opensearchpy/_async/http_aiohttp.py @@ -361,6 +361,7 @@ async def close(self) -> Any: """ if self.session: await self.session.close() + self.session = None async def _create_aiohttp_session(self) -> Any: """Creates an aiohttp.ClientSession(). This is delayed until diff --git a/opensearchpy/connection/http_async.py b/opensearchpy/connection/http_async.py index 468f3244..f5a4ec7c 100644 --- a/opensearchpy/connection/http_async.py +++ b/opensearchpy/connection/http_async.py @@ -277,6 +277,7 @@ async def close(self) -> Any: """ if self.session: await self.session.close() + self.session = None async def _create_aiohttp_session(self) -> Any: """Creates an aiohttp.ClientSession(). This is delayed until From 6f4adff75741b4ce4c992d0c85b7abd98d7e2813 Mon Sep 17 00:00:00 2001 From: odlmarce Date: Thu, 21 Dec 2023 04:01:20 +0100 Subject: [PATCH 02/10] add test Signed-off-by: odlmarce --- test_opensearchpy/test_async/test_server/test_clients.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test_opensearchpy/test_async/test_server/test_clients.py b/test_opensearchpy/test_async/test_server/test_clients.py index cee6bc7b..c2dd32ae 100644 --- a/test_opensearchpy/test_async/test_server/test_clients.py +++ b/test_opensearchpy/test_async/test_server/test_clients.py @@ -67,3 +67,8 @@ async def test_aiohttp_connection_works_without_yarl( resp = await async_client.info(pretty=True) assert isinstance(resp, dict) + +class TestClose: + async def test_close_doesnt_break_client(self, async_client: Any) -> None: + await async_client.close() + await async_client.cluster.health() From f9c1ceda7622b79ced40c9892f7cc500bb4d8280 Mon Sep 17 00:00:00 2001 From: odlmarce Date: Thu, 21 Dec 2023 04:59:26 +0100 Subject: [PATCH 03/10] update changelog Signed-off-by: odlmarce --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b587a75..596e6b38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Added a log collection guide ([#579](https://github.com/opensearch-project/opensearch-py/pull/579)) - Added GHA release ([#614](https://github.com/opensearch-project/opensearch-py/pull/614)) ### Changed +- `AsyncOpensearch` can now be re-used after calling `close()` ([#635](https://github.com/opensearch-project/opensearch-py/pull/635)) ### Deprecated ### Removed - Removed unnecessary `# -*- coding: utf-8 -*-` headers from .py files ([#615](https://github.com/opensearch-project/opensearch-py/pull/615), [#617](https://github.com/opensearch-project/opensearch-py/pull/617)) From 22803d57782ac372540f9f55fd4c99a2edc32493 Mon Sep 17 00:00:00 2001 From: odlmarce Date: Thu, 21 Dec 2023 05:10:04 +0100 Subject: [PATCH 04/10] update changelog + format Signed-off-by: odlmarce --- CHANGELOG.md | 2 +- test_opensearchpy/test_async/test_server/test_clients.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 596e6b38..6568f014 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Added a log collection guide ([#579](https://github.com/opensearch-project/opensearch-py/pull/579)) - Added GHA release ([#614](https://github.com/opensearch-project/opensearch-py/pull/614)) ### Changed -- `AsyncOpensearch` can now be re-used after calling `close()` ([#635](https://github.com/opensearch-project/opensearch-py/pull/635)) +- `AsyncOpensearch` can now be re-used after calling `close()` ([#639](https://github.com/opensearch-project/opensearch-py/pull/639)) ### Deprecated ### Removed - Removed unnecessary `# -*- coding: utf-8 -*-` headers from .py files ([#615](https://github.com/opensearch-project/opensearch-py/pull/615), [#617](https://github.com/opensearch-project/opensearch-py/pull/617)) diff --git a/test_opensearchpy/test_async/test_server/test_clients.py b/test_opensearchpy/test_async/test_server/test_clients.py index c2dd32ae..64f7542c 100644 --- a/test_opensearchpy/test_async/test_server/test_clients.py +++ b/test_opensearchpy/test_async/test_server/test_clients.py @@ -68,6 +68,7 @@ async def test_aiohttp_connection_works_without_yarl( resp = await async_client.info(pretty=True) assert isinstance(resp, dict) + class TestClose: async def test_close_doesnt_break_client(self, async_client: Any) -> None: await async_client.close() From e50f6be81499d96f3144543be356616e0e3ad64e Mon Sep 17 00:00:00 2001 From: odlmarce Date: Thu, 21 Dec 2023 20:32:10 +0100 Subject: [PATCH 05/10] update changelog Signed-off-by: odlmarce --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6568f014..6c59ac60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,12 +8,12 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Added a log collection guide ([#579](https://github.com/opensearch-project/opensearch-py/pull/579)) - Added GHA release ([#614](https://github.com/opensearch-project/opensearch-py/pull/614)) ### Changed -- `AsyncOpensearch` can now be re-used after calling `close()` ([#639](https://github.com/opensearch-project/opensearch-py/pull/639)) ### Deprecated ### Removed - Removed unnecessary `# -*- coding: utf-8 -*-` headers from .py files ([#615](https://github.com/opensearch-project/opensearch-py/pull/615), [#617](https://github.com/opensearch-project/opensearch-py/pull/617)) ### Fixed - Fix KeyError when scroll return no hits ([#616](https://github.com/opensearch-project/opensearch-py/pull/616)) +- Fix reuse of `AsyncOpenSearch` after calling `close` ([#639](https://github.com/opensearch-project/opensearch-py/pull/639)) ### Security ### Dependencies - Bumps `pytest-asyncio` from <=0.21.1 to <=0.23.2 From 62f7d73ce00e7ae9af0aee0b245e55b0b2601d34 Mon Sep 17 00:00:00 2001 From: odlmarce Date: Thu, 21 Dec 2023 22:04:36 +0100 Subject: [PATCH 06/10] add tests using `with` and synchronous client Signed-off-by: odlmarce --- .../test_async/test_server/test_clients.py | 5 +++++ test_opensearchpy/test_server/test_clients.py | 12 ++++++++++++ 2 files changed, 17 insertions(+) diff --git a/test_opensearchpy/test_async/test_server/test_clients.py b/test_opensearchpy/test_async/test_server/test_clients.py index 64f7542c..ab145c2f 100644 --- a/test_opensearchpy/test_async/test_server/test_clients.py +++ b/test_opensearchpy/test_async/test_server/test_clients.py @@ -73,3 +73,8 @@ class TestClose: async def test_close_doesnt_break_client(self, async_client: Any) -> None: await async_client.close() await async_client.cluster.health() + + async with async_client: + await async_client.cluster.health() + async with async_client: + await async_client.cluster.health() diff --git a/test_opensearchpy/test_server/test_clients.py b/test_opensearchpy/test_server/test_clients.py index e945b69a..6ce50fd0 100644 --- a/test_opensearchpy/test_server/test_clients.py +++ b/test_opensearchpy/test_server/test_clients.py @@ -49,3 +49,15 @@ def test_bulk_works_with_bytestring_body(self) -> None: self.assertFalse(response["errors"]) self.assertEqual(1, len(response["items"])) + + +class TestClose(OpenSearchTestCase): + def test_close_doesnt_break_client(self) -> None: + self.client.cluster.health() + self.client.close() + self.client.cluster.health() + + with self.client as client: + client.cluster.health() + with self.client as client: + client.cluster.health() From 7b221e94116075c67e55350009bdd1ae27890721 Mon Sep 17 00:00:00 2001 From: odlmarce Date: Thu, 21 Dec 2023 22:12:07 +0100 Subject: [PATCH 07/10] fix `urllib3.exceptions.ClosedPoolError` breaking synchronous client after `close` Signed-off-by: odlmarce --- opensearchpy/connection/http_urllib3.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/opensearchpy/connection/http_urllib3.py b/opensearchpy/connection/http_urllib3.py index 54f2a22a..b4b2d153 100644 --- a/opensearchpy/connection/http_urllib3.py +++ b/opensearchpy/connection/http_urllib3.py @@ -214,9 +214,14 @@ def __init__( if pool_maxsize and isinstance(pool_maxsize, int): kw["maxsize"] = pool_maxsize - self.pool = pool_class( - self.hostname, port=self.port, timeout=self.timeout, **kw - ) + def urllib3_pool_factory() -> Any: + return pool_class(self.hostname, port=self.port, timeout=self.timeout, **kw) + + self._urllib3_pool_factory = urllib3_pool_factory + self._create_urllib3_pool() + + def _create_urllib3_pool(self) -> None: + self.pool = self._urllib3_pool_factory() def perform_request( self, @@ -228,6 +233,10 @@ def perform_request( ignore: Collection[int] = (), headers: Optional[Mapping[str, str]] = None, ) -> Any: + if self.pool is None: + self._create_urllib3_pool() + assert self.pool is not None + url = self.url_prefix + url if params: url = "%s?%s" % (url, urlencode(params)) @@ -305,4 +314,6 @@ def close(self) -> None: """ Explicitly closes connection """ - self.pool.close() + if self.pool: + self.pool.close() + self.pool = None From 9193f3f0a7a364c9f3338592e4a0a5cf9b582256 Mon Sep 17 00:00:00 2001 From: odlmarce Date: Thu, 21 Dec 2023 22:17:48 +0100 Subject: [PATCH 08/10] update changelog Signed-off-by: odlmarce --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c59ac60..835c403d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Removed unnecessary `# -*- coding: utf-8 -*-` headers from .py files ([#615](https://github.com/opensearch-project/opensearch-py/pull/615), [#617](https://github.com/opensearch-project/opensearch-py/pull/617)) ### Fixed - Fix KeyError when scroll return no hits ([#616](https://github.com/opensearch-project/opensearch-py/pull/616)) -- Fix reuse of `AsyncOpenSearch` after calling `close` ([#639](https://github.com/opensearch-project/opensearch-py/pull/639)) +- Fix reuse of `OpenSearch` using `Urllib3HttpConnection` and `AsyncOpenSearch` after calling `close` ([#639](https://github.com/opensearch-project/opensearch-py/pull/639)) ### Security ### Dependencies - Bumps `pytest-asyncio` from <=0.21.1 to <=0.23.2 From 67ac210dc604d61aef90f974ef2c2773bbc8c1d6 Mon Sep 17 00:00:00 2001 From: odlmarce Date: Sat, 30 Dec 2023 15:57:07 +0100 Subject: [PATCH 09/10] separate tests Signed-off-by: odlmarce --- test_opensearchpy/test_async/test_server/test_clients.py | 9 +++++---- test_opensearchpy/test_server/test_clients.py | 8 ++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/test_opensearchpy/test_async/test_server/test_clients.py b/test_opensearchpy/test_async/test_server/test_clients.py index ab145c2f..521f0600 100644 --- a/test_opensearchpy/test_async/test_server/test_clients.py +++ b/test_opensearchpy/test_async/test_server/test_clients.py @@ -71,10 +71,11 @@ async def test_aiohttp_connection_works_without_yarl( class TestClose: async def test_close_doesnt_break_client(self, async_client: Any) -> None: + await async_client.cluster.health() await async_client.close() await async_client.cluster.health() - async with async_client: - await async_client.cluster.health() - async with async_client: - await async_client.cluster.health() + async def test_with_doesnt_break_client(self, async_client: Any) -> None: + for _ in range(2): + async with async_client as client: + await client.cluster.health() diff --git a/test_opensearchpy/test_server/test_clients.py b/test_opensearchpy/test_server/test_clients.py index 6ce50fd0..a77b0f37 100644 --- a/test_opensearchpy/test_server/test_clients.py +++ b/test_opensearchpy/test_server/test_clients.py @@ -57,7 +57,7 @@ def test_close_doesnt_break_client(self) -> None: self.client.close() self.client.cluster.health() - with self.client as client: - client.cluster.health() - with self.client as client: - client.cluster.health() + def test_with_doesnt_break_client(self) -> None: + for _ in range(2): + with self.client as client: + client.cluster.health() From d4edaa1a1f51131b47767d56f8bf810890672e10 Mon Sep 17 00:00:00 2001 From: odlmarce Date: Sat, 30 Dec 2023 15:57:50 +0100 Subject: [PATCH 10/10] refactor pool factory as lambda Signed-off-by: odlmarce --- opensearchpy/connection/http_urllib3.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/opensearchpy/connection/http_urllib3.py b/opensearchpy/connection/http_urllib3.py index b4b2d153..ab9a1a78 100644 --- a/opensearchpy/connection/http_urllib3.py +++ b/opensearchpy/connection/http_urllib3.py @@ -214,14 +214,13 @@ def __init__( if pool_maxsize and isinstance(pool_maxsize, int): kw["maxsize"] = pool_maxsize - def urllib3_pool_factory() -> Any: - return pool_class(self.hostname, port=self.port, timeout=self.timeout, **kw) - - self._urllib3_pool_factory = urllib3_pool_factory + self._urllib3_pool_factory = lambda: pool_class( + self.hostname, port=self.port, timeout=self.timeout, **kw + ) self._create_urllib3_pool() def _create_urllib3_pool(self) -> None: - self.pool = self._urllib3_pool_factory() + self.pool = self._urllib3_pool_factory() # type: ignore def perform_request( self,