-
Notifications
You must be signed in to change notification settings - Fork 24.6k
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
Indices API: Fixed backward compatibility issue #8387
Conversation
@Override | ||
public void onFailure(Throwable e) { | ||
Throwable rootCause = e; | ||
if (rootCause instanceof RemoteTransportException) { |
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.
Maybe use ExceptionsHelper#unwrapCause
?
Left a few comments, nice work @colings86 ! |
assertThat(settingsMap.size(), equalTo(1)); | ||
Settings settings = settingsMap.get("test"); | ||
assertThat(settings, notNullValue()); | ||
assertThat(settings.get("index.number_of_shards"), equalTo("1")); |
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.
FYI if you want (not that it is a problem as it works now) you can leave the randomized number of shards when creating the index and retrieve it through getNumShards("test").numShards
which you can use for the comparison.
} | ||
} | ||
GetIndexResponse response = new GetIndexResponse(indices.toArray(new String[indices.size()]), warmers, mappings, | ||
aliases, indexToSettings); |
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.
all good to pass in null here as parameter I guess?
LGTM |
If a 1.4 node needs to forward a GET index API call to a <1.4 master it cannot use the GET index API endpoints as the master has no knowledge of it. This change detects that the master does not understand the initial request and instead tries it again using the old APIs. If these calls also do not work, an error is returned Closes #8364
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; | ||
import static org.hamcrest.Matchers.*; | ||
|
||
public class GetIndexBackwardsCompatibilityTests extends ElasticsearchBackwardsCompatIntegrationTest { |
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.
can we have this test on master as well please?
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.
+1
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.
added to master in d0da605
If a 1.4 node needs to forward a GET index API call to a <1.4 master it cannot use the GET index API endpoints as the master has no knowledge of it. This change detects that the master does not understand the initial request and instead tries it again using the old APIs. If these calls also do not work, an error is returned
Closes #8364