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

Allow passing versioned media types to 7.x server #63071

Merged
merged 2 commits into from
Oct 2, 2020

Conversation

pgomulka
Copy link
Contributor

@pgomulka pgomulka commented Sep 30, 2020

7.x client can pass media type with a version which will return a 7.x
version of the api in ES 8.
In ES server 7 this media type shoulld be accepted but it serve the same
version of the API (7x)
relates #51816

7.x client can pass media type with a version which will return a 7.x
version of the api in ES 8.
In ES server 7 this media type shoulld be accepted but it serve the same
version of the API (7x)
@pgomulka pgomulka added :Core/Infra/REST API REST infrastructure and utilities v7.10.0 labels Sep 30, 2020
@pgomulka pgomulka self-assigned this Sep 30, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/REST API)

@pgomulka
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/2

*
* @return a media type string without
*/
private static String removeVersionInMediaType(String mediaType) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to wrap this in if(mediaType.contains("vnd.elasticsearch"){ ...} else return mediaType. It is a micro-optimization, but will probably save a few CPU cycles. Also I think you can collapse the two replaceAll into a single line.

Copy link
Contributor

@jakelandis jakelandis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, 1 minor optional comment

Copy link
Member

@jaymode jaymode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pgomulka pgomulka merged commit eb630e5 into elastic:7.x Oct 2, 2020
pgomulka added a commit to pgomulka/elasticsearch that referenced this pull request Nov 23, 2020
a follow up after elastic#63071 where it missed the XContentType.fromMediaType
method.
That method also have to remove the vendor specific substrings
(vnd.elasticsearch+ and compatible-with parameter) from mediaType value

relates elastic#51816
pgomulka added a commit that referenced this pull request Nov 25, 2020
a follow up after #63071 where it missed the XContentType.fromMediaType
method.
That method also have to remove the vendor specific substrings
(vnd.elasticsearch+ and compatible-with parameter) from mediaType value

relates #51816
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/REST API REST infrastructure and utilities Team:Core/Infra Meta label for core/infra team v7.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants