Add Indices Aliases API to the high level REST client by olcbean · Pull Request #27876 · elastic/elasticsearch

olcbean · 2017-12-18T19:48:06Z

Add Indices Aliases API to the high level REST client

Relates to #27205

elasticmachine · 2017-12-18T19:48:08Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

cbuescher · 2018-01-15T14:48:01Z

@elasticmachine test this please

elasticmachine · 2018-01-15T14:48:04Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

cbuescher · 2018-01-15T14:50:05Z

@olcbean thanks for taking this on, I hope I will be able to look at it shortly. In the meanwhile, could you rebase your branch, unfortunately the "core" project was renamed to "server" in the meantime so the changes in core while have conflicts. Hopefully a simple rebase is enough.

olcbean · 2018-01-16T11:14:09Z

@cbuescher thanks for picking this one up! I just rebased

cbuescher · 2018-01-16T11:36:23Z

@elasticmachine test this please

javanna

thanks a lot @olcbean this looks good already, I left some comments but nothing major, once those are addressed I will merge.

javanna · 2018-01-19T14:39:32Z

client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesClient.java

In our SPEC we call this update_aliases. Can you rename these new methods to updateAliases please?

To be honest I struggled with the naming here..
AFAICS the rest specs are referring to it as update_aliases, the IndicesAdminClient uses aliases, ( as the end point itself is aliases ), and the corresponding request and response are respectively IndicesAliasesRequest and IndicesAliasesResponse. So I was somewhat reluctant to bring a new updateAliases into the mix.

yea I hear you. This comes from the fact that Java was always a special client. I think it makes sense to follow the SPEC convention at this point. Although the request and response follow a different one. We can always rename them later.

javanna · 2018-01-19T14:39:51Z

client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java

same rename as above?

javanna · 2018-01-19T14:40:34Z

client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java

testUpdateAliases?

javanna · 2018-01-19T14:43:17Z

client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java

what did you mean to do with the randomization? I wonder if it gets too complicated with multiple randomized actions...

check that 0 and multiple it is still OK. Should I go back to a single action only?

maybe, I mean in this unit test randomizing the request body so heavily may not make a huge difference. I don't have a strong opinion though, what do you think?

javanna · 2018-01-19T14:44:46Z

docs/java-rest/high-level/apis/indices_aliases.asciidoc

here too, let's call it update aliases like in the SPEC?

javanna · 2018-01-19T14:52:36Z

server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java

should this be builder.array ?

javanna · 2018-01-19T14:57:46Z

server/src/test/java/org/elasticsearch/action/admin/indices/alias/AliasActionsTests.java

thanks a lot for adding this, I know the effort it took code-wise :)

javanna · 2018-01-19T15:00:23Z

test/framework/src/main/java/org/elasticsearch/index/alias/RandomAliasActionsGenerator.java

how about folding this into AliasActionsTests and remove randomization from RequestTests? just an idea, let me know what you think.

javanna · 2018-01-19T15:00:44Z

server/src/test/java/org/elasticsearch/action/admin/indices/alias/AliasActionsTests.java

I see some duplication her between the newly added generator and this method. Maybe we can address that too?

javanna · 2018-01-19T15:05:37Z

docs/java-rest/high-level/apis/indices_aliases.asciidoc

I wonder if we should explain here the feature/API itself or rather just mention how to use it and point to the API docs to know more about it. What do you think about rephrasing to just say e.g. "use this to set a routing value" etc. without explaining what routing is in this page?

Do you mean to follow the low level client docs approach? Just say how to use requests / responses with a few examples.
Or do you mean to have a simple example for every supported API with a request / response and a link to the API docs?

besides the link, I wouldn't explain here what the API does and how the different options work. Just how to use the options with the client. At least I think this is what we've done until now. Makes sense?

olcbean · 2018-01-24T09:01:30Z

@javanna I just pushed a commit to address your comments. Can you have a look?

( I did not rename the asciidoc to updateAliases and did not rebase in order to minimize the noise for the review. I will do that in the last commit )

javanna

I left two tiny comments, LGTM otherwise.

javanna · 2018-01-24T09:28:54Z

docs/java-rest/high-level/apis/indices_aliases.asciidoc

here can we just describe how to add a filter rather than describing what filters do and where they get applied?

javanna · 2018-01-24T09:29:23Z

docs/java-rest/high-level/apis/indices_aliases.asciidoc

s/assosiated/associated

olcbean · 2018-01-24T12:19:13Z

@javanna have another look?

javanna · 2018-01-24T13:09:20Z

looks good @olcbean , just needs rebasing/merging master in

olcbean · 2018-01-25T07:34:05Z

@javanna I just rebased and changed the docs as well ;)

javanna · 2018-01-25T10:25:57Z

test this please

javanna · 2018-01-25T13:34:14Z

thanks @olcbean !

Relates to #27205

* master: (23 commits) Update Netty to 4.1.16.Final (elastic#28345) Fix peer recovery flushing loop (elastic#28350) REST high-level client: add support for exists alias (elastic#28332) REST high-level client: move to POST when calling API to retrieve which support request body (elastic#28342) Add Indices Aliases API to the high level REST client (elastic#27876) Java Api clean up: remove deprecated `isShardsAcked` (elastic#28311) [Docs] Fix explanation for `from` and `size` example (elastic#28320) Adapt bwc version after backport elastic#28358 Always return the after_key in composite aggregation response (elastic#28358) Adds test name to MockPageCacheRecycler exception (elastic#28359) Adds a note in the `terms` aggregation docs regarding pagination (elastic#28360) [Test] Fix DiscoveryNodesTests.testDeltas() (elastic#28361) Update packaging tests to work with meta plugins (elastic#28336) Remove Painless Type from MethodWriter in favor of Java Class. (elastic#28346) [Doc] Fixs typo in reverse-nested-aggregation.asciidoc (elastic#28348) Reindex: Shore up rethrottle test Only assert single commit iff index created on 6.2 isHeldByCurrentThread should return primitive bool [Docs] Clarify `html` encoder in highlighting.asciidoc (elastic#27766) Fix GeoDistance query example (elastic#28355) ...

olcbean force-pushed the hlrc_indices_aliases branch from 1c47099 to 7e03951 Compare December 18, 2017 22:03

cbuescher added review v7.0.0 v6.3.0 labels Jan 15, 2018

cbuescher self-requested a review January 15, 2018 14:47

cbuescher self-assigned this Jan 15, 2018

olcbean force-pushed the hlrc_indices_aliases branch from 7e03951 to 9257fc7 Compare January 16, 2018 11:10

javanna mentioned this pull request Jan 16, 2018

Java high-level REST client completeness #27205

Closed

80 tasks

javanna self-requested a review January 19, 2018 15:08

javanna assigned javanna and unassigned cbuescher Jan 19, 2018

javanna removed the request for review from cbuescher January 19, 2018 15:08

javanna approved these changes Jan 19, 2018

View reviewed changes

javanna approved these changes Jan 24, 2018

View reviewed changes

olcbean added 4 commits January 24, 2018 21:59

add indices aliases api to the high level REST client

ba74972

addressing reviewer comments

8f4294b

docs changes

f0cfab8

docs renamed ( indices_aliases -> update_aliases )

28a8dc6

olcbean force-pushed the hlrc_indices_aliases branch from bf3d6b6 to 28a8dc6 Compare January 25, 2018 07:32

javanna merged commit 9db23e4 into elastic:master Jan 25, 2018

javanna added the backport pending label Jan 25, 2018

javanna pushed a commit that referenced this pull request Jan 25, 2018

Add Indices Aliases API to the high level REST client (#27876)

63500f7

Relates to #27205

javanna removed the backport pending label Jan 25, 2018

romseygeek mentioned this pull request Jan 31, 2018

Move leftover aliases test from core/ to server/ #28463

Merged

jpountz added the >feature label Jun 13, 2018

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

Conversation

olcbean commented Dec 18, 2017

Uh oh!

elasticmachine commented Dec 18, 2017

Uh oh!

cbuescher commented Jan 15, 2018

Uh oh!

elasticmachine commented Jan 15, 2018

Uh oh!

cbuescher commented Jan 15, 2018

Uh oh!

olcbean commented Jan 16, 2018

Uh oh!

cbuescher commented Jan 16, 2018

Uh oh!

javanna left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

olcbean commented Jan 24, 2018

Uh oh!

javanna left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

olcbean commented Jan 24, 2018

Uh oh!

javanna commented Jan 24, 2018

Uh oh!

olcbean commented Jan 25, 2018

Uh oh!

javanna commented Jan 25, 2018

Uh oh!

javanna commented Jan 25, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants