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

Support reindex as asynchronous task #550

Merged

Conversation

kghatton
Copy link

@kghatton kghatton commented Jul 6, 2017

When the WaitForCompletion parameter is set to true, the response from ElasticSearch is in a different format, as it just contains a task id. This PR provides a Start method on the ReindexService that will return the task id instead of the BulkIndexByScrollResponse, which is not appropriate for asynchronous requests. (I believe this could also be appropriate for the other Elastic APIs that have a WaitForCompletion parameter, although my present use case does not extend to them).

Secondly, the TasksListService does not work as expected with single taskId and nodeId values, so, for the simple case of monitoring a single task, I have added a TasksGetTaskService.

@olivere
Copy link
Owner

olivere commented Jul 6, 2017

Thanks for contributing to Elastic. The PR is looking good. Give me some time to make a few more tests. But I hope it'll make it into the next release.

@olivere olivere merged commit 6aa4cc3 into olivere:release-branch.v5 Jul 18, 2017
olivere added a commit that referenced this pull request Jul 18, 2017
This commit adds to PR #550 and changes a few things.

First, the Reindex.Start method is renamed to DoAsync to reflect that
Do starts the request. Reindex.DoAsync starts reindexing in the
background and returns a task id, which can be watched via the Task Get
API, which was also added in #550.

Furthermore, it adds some tests and missing fields in the response
types, and fixes a typo or two in the JSON structs.

See #550
@olivere
Copy link
Owner

olivere commented Jul 18, 2017

I changed a few things in your original PR. See 3116eca for a summary.

Most noticeable is that I renamed Start to DoAsync. I want users to remember that Do... executes the request.

@olivere
Copy link
Owner

olivere commented Jul 18, 2017

Forgot: This will make it into 5.0.42, which I'll release once CI is green.

@kghatton
Copy link
Author

👍

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.

None yet

2 participants