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

[BUG] Elasticsearch RestHighLevelClient can not get response of Info API from OpenSearch server due to missing "tagline" field in MainResponse #901

Closed
tlfeng opened this issue Jun 29, 2021 · 8 comments
Labels
backwards-compatibility Beta bug Something isn't working v1.0.0 Version 1.0.0

Comments

@tlfeng
Copy link
Collaborator

tlfeng commented Jun 29, 2021

Describe the bug

Source: https://discuss.opendistrocommunity.dev/t/spring-data-integration/6239/4

Because the "tagline" field is removed from "MainResponse" in both sever and RHLC side of OpenSearch in PR #427, Info API calls from the existing Elasticsearch Rest-High-Level-Client (such as version 7.10.2) can not get the required "tagline" field from OpenSearch server (version 1.0.0). It causes compatibility issues with some libraries that support Elasticsearch.

For example, SpringData for Elasticsearch uses elasticsearch-rest-high-level-client library to call org.elasticsearch.client.RestHighLevelClient.info() to retrieve the version of the ES cluster in this line.
"tagline" is a required field in the response of that call, without the field, there will be an error.

To Reproduce
Steps to reproduce the behavior:
No detailed steps, but use "SpringData for Elasticsearch" with an OpenSearch 1.0.0 cluster.

Expected behavior

  • "SpringData for Elasticsearch" version 4.x can be compatible with OpenSearch 1.0.0 with the functionality made for Elasticsearch 7.10.2 .
  • Anything that uses the Elasticsearch RHLC library can get response of "Info API" from OpenSearch 1.0.0 clusters.

Plugins
No plugins enabled.

Screenshots
(none)

Host/Environment (please complete the following information):
Platform-independent

Additional context
The issue of removing the tagline: #362
The issue of requesting support for "Spring Data for Elasticsearch": #542

@tlfeng tlfeng added bug Something isn't working untriaged Beta labels Jun 29, 2021
@tlfeng tlfeng changed the title [BUG] OpenSearch server can not connect to Elasticsearch client due to missing "tagline" field in Main Response [BUG] OpenSearch server can not connect to Elasticsearch RestHighLevelClient due to missing "tagline" field in Main Response Jun 29, 2021
@tlfeng tlfeng removed the untriaged label Jun 29, 2021
@tlfeng tlfeng changed the title [BUG] OpenSearch server can not connect to Elasticsearch RestHighLevelClient due to missing "tagline" field in Main Response [BUG] Elasticsearch RestHighLevelClient can not connect to OpenSearch server due to missing "tagline" field in Main Response Jun 29, 2021
@tlfeng
Copy link
Collaborator Author

tlfeng commented Jun 29, 2021

The possible solution is to revert PR #362, but with a different value of "tagline" field. In addition, the "tagline" field can consider to be shown only in the "compatibility mode" (which is introduced in the PR #898).

@tlfeng
Copy link
Collaborator Author

tlfeng commented Jun 29, 2021

Hi @mch2 and @CEHENKLE , what's your idea of putting the tagline in "compatibility mode" or not?

@CEHENKLE
Copy link
Member

let's use "The OpenSearch Project: https://opensearch.org/" as the tag line for now. I'm sure somebody can come up with something better in the future :)

@mch2
Copy link
Member

mch2 commented Jun 29, 2021

@tlfeng imho we should set the tag line in all cases. I think it would be a bit odd to have this tied to a 'compatibility mode' and the value is not the previous tag line. Unless we are totally against having a tag line going forward.

@CEHENKLE
Copy link
Member

I think the reason to put into compatibility mode is to mitigate the risk we might break something that's now NOT expecting a tagline (post RC-1), But I think that's a relatively low probability*, so not worth adding the complexity. Long story short, let's not put it behind the compatibility mode unless someone has a good reason to do it that way.

(*she said, famous last wordsingly).

@tlfeng
Copy link
Collaborator Author

tlfeng commented Jun 29, 2021

imho we should set the tag line in all cases. I think it would be a bit odd to have this tied to a 'compatibility mode' and the value is not the previous tag line.

Thanks @mch2 for your opinion. 😄 Make sense, we can have the tagline going forward.

think the reason to put into compatibility mode is to mitigate the risk we might break something that's now NOT expecting a tagline (post RC-1)

Thanks @CEHENKLE. 👍 Ah, this is a good point. I got that adding the complexity is not necessary.
While, no matter if it's a low probability 😏, adding the tagline back into both sever and client side will be a breaking change to prevent OpenSearch RestHighLevelClient "post RC-1" connecting to OpenSearch sever before 1.0.0-rc1.
But to reduce the complexity, I will revert the whole change in PR #362.

@tlfeng tlfeng changed the title [BUG] Elasticsearch RestHighLevelClient can not connect to OpenSearch server due to missing "tagline" field in Main Response [BUG] Elasticsearch RestHighLevelClient can not get response of Info API from OpenSearch server due to missing "tagline" field in MainResponse Jun 30, 2021
@tlfeng
Copy link
Collaborator Author

tlfeng commented Jul 1, 2021

"tagline" field has been added back to MainResponse in server side. in PR #913, the issue should be resolved now.
But "tagline" field hasn't been added back to MainResponse in RestHighLevelClient side, and I plan to do it after the change in server side has been backported to 1.0.0 branch, to pass the internal BWC test.

@nknize
Copy link
Collaborator

nknize commented Jul 6, 2021

fixed in #913

@nknize nknize closed this as completed Jul 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards-compatibility Beta bug Something isn't working v1.0.0 Version 1.0.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants