Skip to content

Elasticsearch 2.3 new API's feature parity #2017

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

Merged
merged 2 commits into from
Apr 6, 2016
Merged

Elasticsearch 2.3 new API's feature parity #2017

merged 2 commits into from
Apr 6, 2016

Conversation

Mpdreamz
Copy link
Member

@Mpdreamz Mpdreamz commented Apr 6, 2016

This commit adds support for the new 2.3 API's:

  • tasks_list
  • tasks_cancel
  • update_by_query
  • reindex

The spec changes have also been updated in core upstream.

The Reindex API is called ReindexOnServer() to offset it against our
current Reindex() implementation which is still very useful.

Both the reindex and update_by_query steal the status code from one of
the failures and return a valid json body that we need to deserialize.
To support this this AllowedStatusCodes now accepts a -1.

.IsValid for these API's inspects whether any failures have been
returned or not.

Mpdreamz added 2 commits April 4, 2016 17:18
This commit adds support for the new 2.3 API's:

* tasks_list
* tasks_cancel
* update_by_query
* reindex

The spec changes have also been updated in core upstream.

The Reindex API is called `ReindexOnServer()` to offset it against our
current `Reindex()` implementation which is still very useful.

Both the reindex and update_by_query steal the status code from one of
the failures and return a valid json body that we need to deserialize.
To support this this AllowedStatusCodes now accepts a -1.

.IsValid for these API's inspects whether any failures have been
returned or not.
@Mpdreamz Mpdreamz mentioned this pull request Apr 6, 2016
@gmarz
Copy link
Contributor

gmarz commented Apr 6, 2016

LGTM!

To support this this AllowedStatusCodes now accepts a -1.

Maybe add a TODO to make this nullable instead in master?

The Reindex API is called ReindexOnServer() to offset it against our
current Reindex() implementation which is still very useful.

👍 What are your thoughts for 5.0? I'm thinking we should rename ReindexOnServer to Reindex when we remove the old API.

@gmarz gmarz merged commit 05c4b04 into 2.x Apr 6, 2016
@gmarz gmarz deleted the 2.3 branch April 6, 2016 16:16
@Mpdreamz
Copy link
Member Author

Mpdreamz commented Apr 6, 2016

I quite like the idea of keeping both at least untill task result/state are stored in Elasticsearch somehow.

Our roundtripping reindex might still be a lot easier to use and monitor through the non blocking RX observable we expose. Also remapping objects in C# is still alot easier then using scripts which might not even be enabled on the cluster.

@gmarz
Copy link
Contributor

gmarz commented Apr 7, 2016

@Mpdreamz tasks cancel tests are failing in unit test mode. Looks like a setup issue - SetupTaskIds is empty because OnBeforeCall isn't being ran.

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

Successfully merging this pull request may close these issues.

3 participants