From 2edcaf3e95de9396fd446745bddff015d2ab50bd 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 | 37 +++++++++++++++++ .../test_server/test_helpers/test_index.py | 41 +++++++++++++++++++ 6 files changed, 106 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..68da05333 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( + "Settings for %s point to multiple 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..bb1569a49 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( + "Settings for %s point to multiple 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..904ffec9b 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,39 @@ 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) + with pytest.raises(ValidationException) as e: + await i.save() + + message, indices = e.value.args[0][:-1].split(": ") + assert message == "Settings for blog-alias point to multiple indices" + assert set(indices.split(", ")) == {"test-blog-1", "test-blog-2"} diff --git a/test_opensearchpy/test_server/test_helpers/test_index.py b/test_opensearchpy/test_server/test_helpers/test_index.py index 5b8250b42..d5acc1f2b 100644 --- a/test_opensearchpy/test_server/test_helpers/test_index.py +++ b/test_opensearchpy/test_server/test_helpers/test_index.py @@ -26,7 +26,10 @@ from typing import Any +import pytest + from opensearchpy import Date, Document, Index, IndexTemplate, Text +from opensearchpy.exceptions import ValidationException from opensearchpy.helpers import analysis @@ -122,3 +125,41 @@ 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) + with pytest.raises(ValidationException) as e: + i.save() + + message, indices = e.value.args[0][:-1].split(": ") + assert message == "Settings for blog-alias point to multiple indices" + assert set(indices.split(", ")) == {"test-blog-1", "test-blog-2"}