From 1b0440a33f89e733fa716474010e3c9090dc4627 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20Beaul=C3=A9?= Date: Thu, 26 Sep 2024 15:02:13 -0300 Subject: [PATCH] Ignore name of index when fetching settings (#823) 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 ++++++-- .../test_server/test_helpers/test_index.py | 37 +++++++++++++++++ .../test_server/test_helpers/test_index.py | 41 +++++++++++++++++++ 5 files changed, 105 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44af4328..7231eb75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,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 424ca531..68da0533 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 93b86008..bb1569a4 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/test_opensearchpy/test_async/test_server/test_helpers/test_index.py b/test_opensearchpy/test_async/test_server/test_helpers/test_index.py index e2670e55..904ffec9 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 5b8250b4..d5acc1f2 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"}