Skip to content

Commit

Permalink
Ignore name of index when fetching settings
Browse files Browse the repository at this point in the history
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 /<index>/_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é <[email protected]>
  • Loading branch information
tienne-B committed Sep 25, 2024
1 parent a24b9f3 commit 2edcaf3
Show file tree
Hide file tree
Showing 6 changed files with 106 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 13 additions & 4 deletions opensearchpy/_async/helpers/index.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
17 changes: 13 additions & 4 deletions opensearchpy/helpers/index.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion opensearchpy/helpers/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"}
41 changes: 41 additions & 0 deletions test_opensearchpy/test_server/test_helpers/test_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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"}

0 comments on commit 2edcaf3

Please sign in to comment.