From 50771c19dccf098750281f098e7f4f44693e6b1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20Beaul=C3=A9?= Date: Tue, 17 Sep 2024 23:29:39 -0300 Subject: [PATCH] Ignore name of index when fetching settings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit fixes a situation where an index cannot be updated through this client as the index is aliased, with the client pointing to the alias. As the `GET //_settings` request will only ever return the settings for the specified index (through the alias), it would only have one key, so the name of the key would not matter. We can pop the key to get the settings object for the index through the alias. Signed-off-by: Étienne Beaulé --- CHANGELOG.md | 1 + opensearchpy/_async/helpers/index.py | 17 +++++-- opensearchpy/helpers/index.py | 17 +++++-- opensearchpy/helpers/test.py | 2 +- .../test_server/test_helpers/test_index.py | 42 ++++++++++++++++++ .../test_server/test_helpers/test_index.py | 44 +++++++++++++++++++ 6 files changed, 114 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 672fd0653..4a1a45cd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### Removed ### Fixed - Fix `Transport.perform_request`'s arguments `timeout` and `ignore` variable usage ([810](https://github.com/opensearch-project/opensearch-py/pull/810)) +- Fix `Index.save` not passing through aliases to the underlying index ([823](https://github.com/opensearch-project/opensearch-py/pull/823)) ### Security ### Dependencies diff --git a/opensearchpy/_async/helpers/index.py b/opensearchpy/_async/helpers/index.py index 424ca531e..52d7168b2 100644 --- a/opensearchpy/_async/helpers/index.py +++ b/opensearchpy/_async/helpers/index.py @@ -13,7 +13,7 @@ from opensearchpy._async.helpers.search import AsyncSearch from opensearchpy._async.helpers.update_by_query import AsyncUpdateByQuery from opensearchpy.connection.async_connections import get_connection -from opensearchpy.exceptions import IllegalOperation +from opensearchpy.exceptions import IllegalOperation, ValidationException from opensearchpy.helpers import analysis from opensearchpy.helpers.utils import merge @@ -305,9 +305,18 @@ async def save(self, using: Any = None) -> Any: body = self.to_dict() settings = body.pop("settings", {}) analysis = settings.pop("analysis", None) - current_settings = (await self.get_settings(using=using))[self._name][ - "settings" - ]["index"] + + # If _name points to an alias, the response object will contain keys with + # the index name(s) the alias points to. If the alias points to multiple + # indices, raise exception as the intention is ambiguous + settings_response = await self.get_settings(using=using) + if len(settings_response) > 1: + raise ValidationException( + "The settings for %s points to many indices: %s" + % (self._name, ", ".join(list(settings_response.keys()))) + ) + current_settings = settings_response.popitem()[1]["settings"]["index"] + if analysis: if await self.is_closed(using=using): # closed index, update away diff --git a/opensearchpy/helpers/index.py b/opensearchpy/helpers/index.py index 93b860080..8e053b405 100644 --- a/opensearchpy/helpers/index.py +++ b/opensearchpy/helpers/index.py @@ -30,7 +30,7 @@ from opensearchpy.connection.connections import get_connection from opensearchpy.helpers import analysis -from ..exceptions import IllegalOperation +from ..exceptions import IllegalOperation, ValidationException from .mapping import Mapping from .search import Search from .update_by_query import UpdateByQuery @@ -326,9 +326,18 @@ def save(self, using: Optional[OpenSearch] = None) -> Any: body = self.to_dict() settings = body.pop("settings", {}) analysis = settings.pop("analysis", None) - current_settings = self.get_settings(using=using)[self._name]["settings"][ - "index" - ] + + # If _name points to an alias, the response object will contain keys with + # the index name(s) the alias points to. If the alias points to multiple + # indices, raise exception as the intention is ambiguous + settings_response = self.get_settings(using=using) + if len(settings_response) > 1: + raise ValidationException( + "The settings for %s points to many indices: %s" + % (self._name, ", ".join(list(settings_response.keys()))) + ) + current_settings = settings_response.popitem()[1]["settings"]["index"] + if analysis: if self.is_closed(using=using): # closed index, update away diff --git a/opensearchpy/helpers/test.py b/opensearchpy/helpers/test.py index f799bd7d8..2eb27be42 100644 --- a/opensearchpy/helpers/test.py +++ b/opensearchpy/helpers/test.py @@ -34,7 +34,7 @@ from opensearchpy import OpenSearch from opensearchpy.exceptions import ConnectionError -OPENSEARCH_URL = os.environ.get("OPENSEARCH_URL", "https://localhost:9200") +OPENSEARCH_URL = os.environ.get("OPENSEARCH_URL", "http://localhost:9200") def get_test_client(nowait: bool = False, **kwargs: Any) -> OpenSearch: diff --git a/test_opensearchpy/test_async/test_server/test_helpers/test_index.py b/test_opensearchpy/test_async/test_server/test_helpers/test_index.py index e2670e553..f7bd6991c 100644 --- a/test_opensearchpy/test_async/test_server/test_helpers/test_index.py +++ b/test_opensearchpy/test_async/test_server/test_helpers/test_index.py @@ -15,6 +15,7 @@ from opensearchpy import Date, Text from opensearchpy._async.helpers.document import AsyncDocument from opensearchpy._async.helpers.index import AsyncIndex, AsyncIndexTemplate +from opensearchpy.exceptions import ValidationException from opensearchpy.helpers import analysis pytestmark: MarkDecorator = pytest.mark.asyncio @@ -117,3 +118,44 @@ async def test_multiple_indices_with_same_doc_type_work(write_client: Any) -> No assert settings[i]["settings"]["index"]["analysis"] == { "analyzer": {"my_analyzer": {"type": "custom", "tokenizer": "keyword"}} } + + +async def test_index_can_be_saved_through_alias_with_settings( + write_client: Any, +) -> None: + raw_index = AsyncIndex("test-blog", using=write_client) + raw_index.settings(number_of_shards=3, number_of_replicas=0) + raw_index.aliases(**{"blog-alias": {}}) + await raw_index.save() + + i = AsyncIndex("blog-alias", using=write_client) + i.settings(number_of_replicas=1) + await i.save() + + settings = await write_client.indices.get_settings(index="test-blog") + assert "1" == settings["test-blog"]["settings"]["index"]["number_of_replicas"] + + +async def test_validation_alias_has_many_indices(write_client: Any) -> None: + raw_index_1 = AsyncIndex("test-blog-1", using=write_client) + raw_index_1.settings(number_of_shards=3, number_of_replicas=0) + raw_index_1.aliases(**{"blog-alias": {}}) + await raw_index_1.save() + + raw_index_2 = AsyncIndex("test-blog-2", using=write_client) + raw_index_2.settings(number_of_shards=3, number_of_replicas=0) + raw_index_2.aliases(**{"blog-alias": {}}) + await raw_index_2.save() + + i = AsyncIndex("blog-alias", using=write_client) + try: + await i.save() + except ValidationException as e: + message, indices = e.args[0].split(': ') + assert ( + message + == "The settings for blog-alias points to many indices" + ) + assert set(indices.split(", ")) == {"test-blog-1", "test-blog-2"} + return + assert False diff --git a/test_opensearchpy/test_server/test_helpers/test_index.py b/test_opensearchpy/test_server/test_helpers/test_index.py index 5b8250b42..9ef504dcf 100644 --- a/test_opensearchpy/test_server/test_helpers/test_index.py +++ b/test_opensearchpy/test_server/test_helpers/test_index.py @@ -27,6 +27,7 @@ from typing import Any from opensearchpy import Date, Document, Index, IndexTemplate, Text +from opensearchpy.exceptions import ValidationException from opensearchpy.helpers import analysis @@ -122,3 +123,46 @@ def test_multiple_indices_with_same_doc_type_work(write_client: Any) -> None: assert settings[j]["settings"]["index"]["analysis"] == { "analyzer": {"my_analyzer": {"type": "custom", "tokenizer": "keyword"}} } + + +def test_index_can_be_saved_through_alias_with_settings(write_client: Any) -> None: + raw_index = Index("test-blog", using=write_client) + raw_index.settings(number_of_shards=3, number_of_replicas=0) + raw_index.aliases(**{"blog-alias": {}}) + raw_index.save() + + i = Index("blog-alias", using=write_client) + i.settings(number_of_replicas=1) + i.save() + + assert ( + "1" + == raw_index.get_settings()["test-blog"]["settings"]["index"][ + "number_of_replicas" + ] + ) + + +def test_validation_alias_has_many_indices(write_client: Any) -> None: + raw_index_1 = Index("test-blog-1", using=write_client) + raw_index_1.settings(number_of_shards=3, number_of_replicas=0) + raw_index_1.aliases(**{"blog-alias": {}}) + raw_index_1.save() + + raw_index_2 = Index("test-blog-2", using=write_client) + raw_index_2.settings(number_of_shards=3, number_of_replicas=0) + raw_index_2.aliases(**{"blog-alias": {}}) + raw_index_2.save() + + i = Index("blog-alias", using=write_client) + try: + i.save() + except ValidationException as e: + message, indices = e.args[0].split(': ') + assert ( + message + == "The settings for blog-alias points to many indices" + ) + assert set(indices.split(", ")) == {"test-blog-1", "test-blog-2"} + return + assert False