-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
Fix reindex with transport client #19997
Conversation
@dadoonet, the bulk of this reworks some changes you made because they didn't play well with the transport client. Can you have a look? |
Looks like @dadoonet can't review it for a while. So, anyone else want it? |
@elasticmachine, retest this |
Test failure is caused by the branch point, not the change itself. |
0bdbb4d
to
5dc8618
Compare
Rebased. That should help. |
import org.elasticsearch.test.transport.AssertingLocalTransport; | ||
import org.elasticsearch.transport.Transport; | ||
import org.elasticsearch.transport.TransportService; | ||
|
||
import java.util.Collections; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No code was added to this test class, only removed, so is this added import necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind, I see, it was a remnant from the imports up top, just got shifted around
LGTM |
5dc8618
to
3483352
Compare
The big change here is cleaning up the `TaskListResponse` so it doesn't have a breaky `toString` implementation. That was causing the reindex tests to break. Also removed `NetworkModule#registerTaskStatus` which is part of the Plugin API. Use `Plugin#getNamedWriteables` instead.
3483352
to
fdd5061
Compare
Merged! Thanks @abeyad ! |
Fixes reindex's status requests through the transport client and reworks
ListTasksResponse
so itstoString
isn't breaky with the transport client.Reworks #19773
Closes #19979