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

HLRC for _mtermvectors #35266

Merged

Conversation

mayya-sharipova
Copy link
Contributor

relates to #27205, #33447

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

Copy link
Contributor

@hub-cap hub-cap left a comment

Choose a reason for hiding this comment

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

Great work! Im approving so i dont need to be pinged again, but I just want to make sure you follow up with @jtibshirani WRT the _type stuff

@@ -197,6 +220,9 @@ public boolean getRealtime() {
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
builder.field("_index", index);
builder.field("_type", type);
Copy link
Contributor

Choose a reason for hiding this comment

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

I know @jtibshirani has done some work on removing types from 7.0, and I dont know how this may or may not be impacted. @jtibshirani does this need to be different at all based on the work you did in #35421 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the ping! I don't think we need to worry about it in this PR. We'll do a separate pass through each affected API to add the right deprecations (including the Java HLRC).

@@ -670,7 +670,6 @@ public void testApiNamingConventions() throws Exception {
"indices.exists_type",
"indices.get_upgrade",
"indices.put_alias",
"mtermvectors",
Copy link
Contributor

Choose a reason for hiding this comment

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

yay!

:api: multi-term-vectors
:request: MultiTermVectorsRequest
:response: MultiTermVectorsResponse
--
Copy link
Contributor

Choose a reason for hiding this comment

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

you can stick things like :tvrequest: TermVectorsRequest in here as well. Of course call it what ever you want :)

@mayya-sharipova mayya-sharipova merged commit aaeb47d into elastic:master Nov 19, 2018
mayya-sharipova added a commit to mayya-sharipova/elasticsearch that referenced this pull request Nov 19, 2018
mayya-sharipova added a commit that referenced this pull request Nov 19, 2018
mayya-sharipova added a commit that referenced this pull request Nov 19, 2018
mayya-sharipova added a commit that referenced this pull request Nov 19, 2018
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants