[BUG] Missing type and settings parameters in _sync/snapshots & _async/snapshots create_respository methods

[berglh]

Elasticsearch version: 8.11.1

elasticsearch-py version: 8.13.0

I understand these versions are different, but I think I've noticed a problem in commits on the snapshots create_repository method.

Description of the problem including expected versus actual behaviour:

I am using the Elasticsearch Python client to create a repository in our Elasticsearch deployments in Elastic Cloud Enterprise 3.6.0. The repository storage target is an on-premises Dell ECS appliance that's compatible with S3 protocol. We deploy the configuration for our Elasticsearch cluster (index templates, pipelines, repository etc) using Gitlab CI on a push to our main branch. The first stage of deployment is to test deployment to Elasticsearch in a Docker container on a GitLab Runner host. Since version 8.13.0 of the Python Elasticsearch client, this is now failing to create the repository with the following traceback:

Traceback (most recent call last):
  File "datastreams/modules/repositories.py", line 32, in put_all_repo
    resp = es.snapshot.create_repository(**repos[r])
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/berg/.local/lib/python3.11/site-packages/elasticsearch/_sync/client/utils.py", line 446, in wrapped
    return api(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^
TypeError: SnapshotClient.create_repository() got an unexpected keyword argument 'type'

Steps to reproduce:

We store the configuration in Git and supply this payload as the configuration to the create_repository method.

Payload

{
  "name": "snapshot_name",
  "type": "s3",
  "settings": {
    "endpoint": "https://s3.storage.ourdomain.com",
    "client": "our_dell_ecs",
    "bucket": "OURBUCKET",
    "access_key": "username",
    "secret_key": "secret"
  },
  "verify": false
}

Method

es.snapshot.create_repository(**payload_above)

Investigation:

I looked at the commit history of _sync/client/snapshot.py, and it seems like from the last human commit to the last auto-generated docs commit the parameters for the class method have vanished.

Commit History: https://github.com/elastic/elasticsearch-py/commits/main/elasticsearch/_sync/client/snapshot.py

Latest Commit March 10th:

elasticsearch-py/elasticsearch/_sync/client/snapshot.py

Lines 273 to 288 in 53c5ea3

	def create_repository(
	self,
	*,
	name: str,
	repository: t.Optional[t.Mapping[str, t.Any]] = None,
	body: t.Optional[t.Mapping[str, t.Any]] = None,
	error_trace: t.Optional[bool] = None,
	filter_path: t.Optional[t.Union[str, t.Sequence[str]]] = None,
	human: t.Optional[bool] = None,
	master_timeout: t.Optional[
	t.Union["t.Literal[-1]", "t.Literal[0]", str]
	] = None,
	pretty: t.Optional[bool] = None,
	timeout: t.Optional[t.Union["t.Literal[-1]", "t.Literal[0]", str]] = None,
	verify: t.Optional[bool] = None,
	) -> ObjectApiResponse[t.Any]:

Previous Commit March 7th:

elasticsearch-py/elasticsearch/_sync/client/snapshot.py

Lines 273 to 290 in 45518b0

	def create_repository(
	self,
	*,
	name: str,
	settings: t.Optional[t.Mapping[str, t.Any]] = None,
	type: t.Optional[str] = None,
	error_trace: t.Optional[bool] = None,
	filter_path: t.Optional[t.Union[str, t.Sequence[str]]] = None,
	human: t.Optional[bool] = None,
	master_timeout: t.Optional[
	t.Union["t.Literal[-1]", "t.Literal[0]", str]
	] = None,
	pretty: t.Optional[bool] = None,
	repository: t.Optional[t.Mapping[str, t.Any]] = None,
	timeout: t.Optional[t.Union["t.Literal[-1]", "t.Literal[0]", str]] = None,
	verify: t.Optional[bool] = None,
	body: t.Optional[t.Dict[str, t.Any]] = None,
	) -> ObjectApiResponse[t.Any]:

Looking at the API documentation, there doesn't appear to be any changes to the underlaying Elasticsearch API: https://www.elastic.co/guide/en/elasticsearch/reference/master/put-snapshot-repo-api.html

My guess the commit was squashed/force pushed in such a way that these parameters are missing as we also do not see the lines in the commit diff being removed. I guess could also be lead astray by abstraction with **kwargs parsing happening elsewhere with a closer look. Either way, it isn't working as we would expect vs the Elasticsearch API which has remained static, and the error is thrown by Elasticsearch client parameter validation.

[pquentin]

Hello! And sorry that elasticsearch-py 8.13 broke your workflow. This part of the code is generated from the elasticsearch-specification repository. The current representation was fine for Python which only exposes the first level (type and settings, not settings.endpoint) but caused issues in languages like Java, .NET, C# and JavaScript. elastic/elasticsearch-specification#2255 fixes those issues at the cost of breaking the compatibility with all 8.12 clients.

This means that while this worked in elasticsearch-py 8.12:

client.snapshot.create_repository(
    name="snapshot_name",
    verify=False,
    type="url",
    settings={
        "url": "file:/mount/backups/my_fs_backup_location",
    },
)

This is now needed in elasticsearch-py 8.13:

es.snapshot.create_repository(
    name="snapshot_name",
    verify=False,
    repository={
        "type": "url",
        "settings": {
            "url": "file:/mount/backups/my_fs_backup_location",
        },
    },
)

If you need something that works both in 8.12 and 8.13, you can use the body parameter instead:

es.snapshot.create_repository(
    name="snapshot_name",
    verify=False,
    body={
        "type": "url",
        "settings": {
            "url": "file:/mount/backups/my_fs_backup_location",
        },
    },
)

Sorry again that this breaks your workflow.

[berglh]

@pquentin Thank-you for explaining the reasoning, it seems like a good project to standardise functionality across the various clients. I did receive a warning message on this method about using secrets in the payload instead of the Elasticsearch keystore, but I didn't ever see a deprecation warning in the Gitlab CI output for these parameters.

It would have been nice if we had some kind of heads up from the client about this change in advance, as I'm always looking at this output and looking for warnings/errors. I'm wondering if my inclusion of secrets masked that or if there was no deprecation warning. Either way, thanks for taking the time to point me in the right direction :)

[pquentin]

There was no deprecation warning, you did everything right. I'll try to see how we can add one if we have to do this kind of change in the future.