Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add API compatibility content-type support to HLRC #77859

Closed
swallez opened this issue Sep 16, 2021 · 2 comments · Fixed by #78490
Closed

Add API compatibility content-type support to HLRC #77859

swallez opened this issue Sep 16, 2021 · 2 comments · Fixed by #78490
Assignees
Labels

Comments

@swallez
Copy link
Member

swallez commented Sep 16, 2021

The High Level Rest Client should support the API compatibility content-type.

It should be opt-in, and enabled in two ways:

  • as a configuration on the client itself, e.g. client.setApiCompatibilityMode(true)
  • from an ELASTICSEARCH_APIVERSIONING environment variables as generally supported by Elasticsearch client libraries, which allows enabling compatibility mode without rebuilding applications.

Looking at RequestContervers.REQUEST_BODY_CONTENT_TYPE it seems that HLRC only supports JSON encoding and not CBOR/SMILE/YAML, so we should only support the corresponding vendor media type application/vnd.elasticsearch+smile;compatible-with=7.

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

@joegallo joegallo self-assigned this Sep 16, 2021
@dakrone dakrone assigned dakrone and unassigned joegallo Sep 23, 2021
@swallez
Copy link
Member Author

swallez commented Sep 23, 2021

After a conversation with @dakrone we fine-tuned how this should work with some implementation hints.

TL;DR: LLRC is where it should go, an it's a creation-time setting that cannot be changed dynamically.

I was initially thinking that HLRC was the best place for this since LLRC deals with fully-formed requests and doesn't care much about what they contain. Now looking more closely, the issue is that we need to provide an opt-in configuration along with the env var, and HLRC has no configuration and its methods are only related to API endpoints.

So LLRC may indeed be the place for it in the current HLRC/LLRC design. We can add this in LLRC's performRequest[Async] where we can change the content-type if it's JSON. We can change it directly if the request entity is an AbstractHttpEntity (that seems to always be the case) with a fallback to an HttpEntityWrapper that changes the content-type.

Regarding the new client, this isn't done yet and is the responsibility of the Transport implementation that abstracts the HTTP implementation and JSON encoding/decoding (see the RestClientTransport implementation), so if we implement it in LLRC we kill two birds with one stone and the transport would inherit it from its lower level layer.

And this makes even more sense in the migration scenario where users can use both HLRC and the new client on top of a single LLRC instance as they would only have to take care of compat mode once.

dakrone added a commit to dakrone/elasticsearch that referenced this issue Sep 29, 2021
This adds support for the headers necessary for REST version compatibility to the Low Level Rest
Client (LLRC).

Compatibility mode can be turned on either with the `.setAPICompatibilityMode(true)` when creating
the client, or by setting the `ELASTIC_CLIENT_APIVERSIONING` to `true` similar to our other
Elasticsearch clients.

Resolves elastic#77859
dakrone added a commit that referenced this issue Oct 7, 2021
This adds support for the headers necessary for REST version compatibility to the High Level Rest
Client (HLRC).

Compatibility mode can be turned on either with the .setAPICompatibilityMode(true) when creating
the client, or by setting the ELASTIC_CLIENT_APIVERSIONING to true similar to our other
Elasticsearch clients.

Resolves #77859
dakrone added a commit to dakrone/elasticsearch that referenced this issue Oct 7, 2021
This adds support for the headers necessary for REST version compatibility to the High Level Rest
Client (HLRC).

Compatibility mode can be turned on either with the .setAPICompatibilityMode(true) when creating
the client, or by setting the ELASTIC_CLIENT_APIVERSIONING to true similar to our other
Elasticsearch clients.

Resolves elastic#77859
elasticsearchmachine pushed a commit that referenced this issue Oct 7, 2021
This adds support for the headers necessary for REST version compatibility to the High Level Rest
Client (HLRC).

Compatibility mode can be turned on either with the .setAPICompatibilityMode(true) when creating
the client, or by setting the ELASTIC_CLIENT_APIVERSIONING to true similar to our other
Elasticsearch clients.

Resolves #77859
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants