fix: make unit tests pass again with elasticsearch 8.x client

[lvaylet]

No description provided.

[ocervell]

This prevents to pass additional config params, which can be required for some setups such as connecting with ElasticCloud or to a local Elasticsearch instance and pass basic auth for example:

ElastiCloud:

client = Elasticsearch(
    cloud_id=CLOUD_ID,
    basic_auth=("elastic", ELASTIC_PASSWORD)
)

Basic auth

client = Elasticsearch(
    "https://localhost:9200",
    ca_certs="/path/to/http_ca.crt",
    basic_auth=("elastic", ELASTIC_PASSWORD)
)

Multiple nodes

client = Elasticsearch(
    ["https://localhost:9200", "https://localhost:9201"] ,
    ca_certs="/path/to/http_ca.crt",
    basic_auth=("elastic", ELASTIC_PASSWORD)
)

API Token

client = Elasticsearch(
    "http://localhost:9200",
    api_key=("id", "api_key")
)

The input params of the client are pretty awkard (see here), for instance the basic auth takes a tuple as input as well as the API key, or the URL that can be optional or not makes it a bit more ennoying so I would suggest something like:

import copy

def __init__(self, client=None, **es_config):
    self.client = client
    if self.client is None:
        conf = copy.deepcopy(es_config)
        url = conf.pop('url')
        basic_auth = conf.pop('basic_auth')
        api_key = conf.pop('api_key')
        if url:
          conf['hosts'] = url
        if basic_auth:
          conf['basic_auth'] = (basic_auth['username'], basic_auth['password'])
        if api_key:
          conf['api_key'] = (api_key['id'], api_key['value'])
        self.client = Elasticsearch(**conf)
        

[lvaylet]

Thanks for catching this. I reused your suggested code, slightly updated it to handle missing keys on dict.pop() and added comments to make maintenance easier later on.

Now does that mean that we should replace url with hosts in the configuration files? Version 8.x.x of the client requires either hosts or cloud_id to be specified.

Finally, should I write unit tests to cover all these scenarios (Elastic Cloud, basic auth, multiple nodes, API token...)?

[ocervell]

I think we should maintain the backward compat for this one, so let's keep url for now and we pass it as hosts to the client. We should update our Elasticsearch docs a bit (can be done in a further PR probably) to specify that url can be a single host (string) or a set of hosts (list).

About the unit tests, that's probably a good idea :) maybe split them in a further PR so that we fix the broken unit tests with this PR asap.

[lvaylet]

I'd rather not accumulate technical debt so I updated the docs in this PR. I created a separate issue for the unit tests: #227.

[lvaylet]

Thanks @ocervell for approving the changes. Do you expect me to go ahead and merge the PR myself? If so, which method should I use: "merge commit" or "squash and merge" (my preference)? If "squash and merge", are you happy with the commit message (i.e. the PR title)?