Add support for aliases in elasticsearch document store

[ZeJ0hn]

Proposed changes:

• Add the support for alias in elasticsearch. indices.get() method from elasticsearch python library doesn't return alias name as key, but return indexes grouped by the alias, so we have to loop over all keys to check that every indices are correct

Status (please check what you already did):

• First draft (up for discussions & feedback)
• Final code
• Added tests
• Updated documentation

[CLAassistant]

All committers have signed the CLA.

[julian-risch]

@ZeJ0hn index is not defined here as I found out from the failing test. I think it makes sense to use index_id and maybe also index_name here, right?

[julian-risch]

Already fixed. Wow. 🤩

[ZeJ0hn]

My first pull request in your project so I need to be reactive! 🤓

[julian-risch]

The code changes look very good to me. 👍 What I would like to see added as a nice bonus before we merge this pull request is a small test in test_document_store.py. You can have a look at test_elasticsearch_search_field_mapping in there as a simple example. The test could create an alias of several indices and then initialize an ElasticsearchDocumentStore with that alias as index name. As create_index is True by default, it will internally call self._create_document_index(index) and then we can check whether all indices in the alias fulfill the requirements or raise an Exception. @ZeJ0hn What do you think?

[ZeJ0hn]

Sounds good. I'm going to do that this afternoon

[ZeJ0hn]

Unit test OK:

• one creates a store using 2 valid indices and 1 alias
• second creates a store using 1 valid index, 1 invalid index, and 1 alias: Exception must be raised

[julian-risch]

From the test log I see that the Exception wasn't raised as expected:

        with pytest.raises(Exception):
            # missing field "content" in index "haystack_existing_alias_1"
            ElasticsearchDocumentStore(
>               index="haystack_existing_alias", search_fields=["content"], content_field="title"
            )
E           Failed: DID NOT RAISE <class 'Exception'>

[ZeJ0hn]

Hi @tstadel,

In my case, I don't want all using *. We have many indices with different versions of them (different mappings for an incremental tests). And we use an alias to always point to the last one. That's why we need alias support.

[tstadel]

Hi @tstadel,

In my case, I don't want all using *. We have many indices with different versions of them (different mappings for an incremental tests). And we use an alias to always point to the last one. That's why we need alias support.

@ZeJ0hn Yep, I almost guessed that. I bet that no one uses the grouping function of aliases.But speaking about complexity, because it is technically possible, it's better to support it. If we did not, we at least would need to raise an error or show a warning which again increases complexity. So let's keep it the way it is.
You can however insert a comment in front of the loop like

# Technically it is possible to group multiple indexes to a single alias. As all of them will be searched we have to ensure that all of them meet our requirements.

[julian-risch]

Looks good to me! 👍 It's ready to be merged. I just added the comment about the for loop and all the tests look good to me. The two failing tests have nothing to do with this pull request. So I will go ahead and merge. @ZeJ0hn Thank you so much for this pull request and congratulations to your first contribution to Haystack! 🥇

[julian-risch]

@ZeJ0hn There is one final thing you could do, which is enable all GitHub actions on this pull request. We describe how to do that in detail here: https://github.com/deepset-ai/haystack/blob/master/CONTRIBUTING.md#forks
That would allow automatic code style and documentation updates on your fork.

[ZeJ0hn]

Thank you!