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 20, 2024
1 parent a24b9f3 commit 7da15f6
Show file tree
Hide file tree
Showing 6 changed files with 108 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(

Check warning on line 314 in opensearchpy/_async/helpers/index.py

View check run for this annotation

Codecov / codecov/patch

opensearchpy/_async/helpers/index.py#L312-L314

Added lines #L312 - L314 were not covered by tests
"The settings for %s points to many indices: %s"
% (self._name, ", ".join(list(settings_response.keys())))
)
current_settings = settings_response.popitem()[1]["settings"]["index"]

Check warning on line 318 in opensearchpy/_async/helpers/index.py

View check run for this annotation

Codecov / codecov/patch

opensearchpy/_async/helpers/index.py#L318

Added line #L318 was not covered by tests

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(

Check warning on line 335 in opensearchpy/helpers/index.py

View check run for this annotation

Codecov / codecov/patch

opensearchpy/helpers/index.py#L333-L335

Added lines #L333 - L335 were not covered by tests
"The settings for %s points to many indices: %s"
% (self._name, ", ".join(list(settings_response.keys())))
)
current_settings = settings_response.popitem()[1]["settings"]["index"]

Check warning on line 339 in opensearchpy/helpers/index.py

View check run for this annotation

Codecov / codecov/patch

opensearchpy/helpers/index.py#L339

Added line #L339 was not covered by tests

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")

Check warning on line 37 in opensearchpy/helpers/test.py

View check run for this annotation

Codecov / codecov/patch

opensearchpy/helpers/test.py#L37

Added line #L37 was not covered by tests


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,41 @@ 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
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 @@ -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


Expand Down Expand Up @@ -122,3 +123,43 @@ 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

0 comments on commit 7da15f6

Please sign in to comment.