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 high-level REST client API for _freeze and _unfreeze #35723

Merged
merged 18 commits into from
Nov 28, 2018

Conversation

s1monw
Copy link
Contributor

@s1monw s1monw commented Nov 20, 2018

This change adds support for _freeze and _unfreeze to the HLRC

Relates to #34352

This change adds support for `_freeze` and `_unfreeze` to the HLRC

Relates to elastic#34352
Copy link
Member

@javanna javanna left a comment

Choose a reason for hiding this comment

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

I have left a bunch of minor comments, I think we also need docs for these new hlrc API. @hub-cap and/or @nik9000 can you also look at some of my questions, please?

@@ -403,4 +403,20 @@ static Request analyze(AnalyzeRequest request) throws IOException {
req.setEntity(RequestConverters.createEntity(request, RequestConverters.REQUEST_BODY_CONTENT_TYPE));
return req;
}

static Request freezeIndex(FreezeIndexRequest freezeIndexRequest) {
Copy link
Member

Choose a reason for hiding this comment

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

just double checking that these should be added with all the other OSS API, I think it makes sense but let's be sure.
Does it make sense to add unit testing for these to IndicesRequestConvertersTests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

everything is OSS in this package the serverside might not be

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that its fine that indices type things can be in the IndicesClient, regardless of their status of commercial/noncommercial.

@s1monw
Copy link
Contributor Author

s1monw commented Nov 20, 2018

@javanna I addressed your comments

@s1monw s1monw mentioned this pull request Nov 20, 2018
7 tasks
@s1monw s1monw requested review from hub-cap and nik9000 November 20, 2018 21:04
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.

Agree on the need for docs and the minor comments below.

@@ -403,4 +403,20 @@ static Request analyze(AnalyzeRequest request) throws IOException {
req.setEntity(RequestConverters.createEntity(request, RequestConverters.REQUEST_BODY_CONTENT_TYPE));
return req;
}

static Request freezeIndex(FreezeIndexRequest freezeIndexRequest) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that its fine that indices type things can be in the IndicesClient, regardless of their status of commercial/noncommercial.

@s1monw
Copy link
Contributor Author

s1monw commented Nov 21, 2018

Agree on the need for docs

where are the docs missing?

@s1monw s1monw requested review from hub-cap and javanna November 21, 2018 16:52
@s1monw
Copy link
Contributor Author

s1monw commented Nov 21, 2018

@hub-cap @javanna I pushed new commits

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.

Nits on the docs, and pls move the requests into o.e.c.indices

@s1monw
Copy link
Contributor Author

s1monw commented Nov 26, 2018

@hub-cap I pushed new commits

@s1monw s1monw requested a review from hub-cap November 26, 2018 21:04
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.

lgtm :shipit:

@s1monw s1monw merged commit 89e4ac8 into elastic:master Nov 28, 2018
@s1monw s1monw deleted the frozen_hlrc branch November 28, 2018 14:42
s1monw added a commit that referenced this pull request Nov 28, 2018
This change adds support for `_freeze` and `_unfreeze` to the HLRC

Relates to #34352
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.

4 participants