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

Undocumented breaking change when moving from Rest High Level Client 7.15 to 7.16 #82357

Closed
rdehuyss opened this issue Jan 10, 2022 · 11 comments · Fixed by #83234
Closed

Undocumented breaking change when moving from Rest High Level Client 7.15 to 7.16 #82357

rdehuyss opened this issue Jan 10, 2022 · 11 comments · Fixed by #83234
Assignees
Labels
>bug >docs General docs changes Team:Data Management Meta label for data/management team Team:Docs Meta label for docs team

Comments

@rdehuyss
Copy link

Elasticsearch version (bin/elasticsearch --version): not relevant

Plugins installed: [] not relevant

JVM version (java -version): not relevant

OS version (uname -a if on a Unix-like system): not relevant

Description of the problem including expected versus actual behavior: not relevant

Steps to reproduce:

  1. Use XContentBuilder from elasticsearch-rest-high-level-client version 7.15
  2. Upgrade elasticsearch-rest-high-level-client to version 7.16
  3. Breaking imports

Provide logs (if relevant):
See https://www.elastic.co/guide/en/elasticsearch/reference/current/migrating-7.16.html#breaking-changes-7.16. There is nothing mentioned regarding XContentBuilder.

@rdehuyss rdehuyss added >bug needs:triage Requires assignment of a team area label labels Jan 10, 2022
@rdehuyss
Copy link
Author

See also jobrunr/jobrunr#291

@ChrisHegarty
Copy link
Contributor

The transitive dependency from HLRC onto xcontent exposes the xcontent API to Java developers. It may be worth noting the change in package of xcontent in the 7.16 breaking changes.

@costin Is the xcontent API expected to be usable and stable across minor releases, when used in conjunction with the HLRC ?

@rdehuyss
Copy link
Author

Hi @ChrisHegarty , @costin ,

this used to be one of the recommended ways to create documents that can be indexed by Elastic.

See https://www.elastic.co/guide/en/elasticsearch/client/java-rest/current/java-rest-high-document-index.html and search for XContentBuilder.

For https://github.com/jobrunr/jobrunr it results in the fact that users cannot update their Elastic Client Jar.

Kind regards,
Ronald

@costin
Copy link
Member

costin commented Jan 10, 2022

Hi @rdehuyss
Sorry to hear about your troubles. This is indeed a breaking change that should be documented appropriately.

@ChrisHegarty In general, outside the REST endpoints I'm not aware of any guarantees around the internal Elasticsearch classes and packages with the notable exception of the plugin API.
XContent is highly used internally and due to historical reasons (such as the high level client depending on the server initially) it started being used within our examples which, as @rdehuyss points out, made it a user facing API.

Note that in ES 8.0, the high level rest client is rewritten from scratch and has no dependency from ES to prevent the problem mentioned above.

@gwbrown gwbrown added :Data Management/Java High Level REST Client >docs General docs changes and removed needs:triage Requires assignment of a team area label labels Jan 10, 2022
@elasticmachine elasticmachine added Team:Docs Meta label for docs team Team:Data Management Meta label for data/management team labels Jan 10, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-docs (Team:Docs)

@ChrisHegarty
Copy link
Contributor

I think that the general situation is worse than ( I ) initially thought. There are java types in public accessible members in the HLRC from server, core, xcontent, to name but a few. Maintaining binary compatibility (as defined and understood in the Java Language Specification) across even minor versions requires that all those types, from the aforementioned modules and libraries, remain effectively frozen and unchanged. Otherwise, a linkage error may occur at runtime. I'm unsure of the requirements or constraints in this area relating to source compatibility, if any.

Given that xcontent was primarily moved so solve a split package issue in 8.x, the motivation for the backport to 7.x was to keep the code in sync. Unfortunately, the change is already in a shipping 7.16 release, so if it is reverted now in 7.17 this could be even more disruptive. But of course, it is possible to restore the xcontent package to its original name in 7.17 if that is deemed necessary. Otherwise, it should be documented as a "breaking change" - if we consider the xcontent API a public supported artefact.

Input will be needed from the Data Management Team on the above options.

@ChrisHegarty
Copy link
Contributor

Given that the change is already in a shipping release 7.16 we feel it too disruptive to revert the package name change. The change will be document as a breaking change introduced in 7.16.

@lockewritesdocs lockewritesdocs self-assigned this Jan 25, 2022
@lockewritesdocs
Copy link
Contributor

@ChrisHegarty, what are the impacts of this change and what actions can they take? Can users migrate to the Java API to avoid this issue? Or is their main option to upgrade to Elasticsearch 8.0, where the high level rest client is rewritten from scratch and has no dependency from ES to prevent the problem as @costin mentioned?

@ChrisHegarty
Copy link
Contributor

The best long term solution is to consider migrating from the High Level Rest Client. However, that is a larger effort which in some cases could be postponed until a later time. So in the shorter term ...

Deveopers/Users maintaining a Java Client application (likely also using the High Level Rest Client API), need to update and recompile their source code to work with 7.17. This is the minimal change that is required.

The source code change required is to simply update the import statements at the top of the Java source code file,
from:
import org.elasticsearch.common.xcontent.xxx
to:
import org.elasticsearch.xcontent.xxx

@lockewritesdocs
Copy link
Contributor

Closed by #83234

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug >docs General docs changes Team:Data Management Meta label for data/management team Team:Docs Meta label for docs team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants