[Versioning] Fix Version.fromString logic for legacy version #604
Conversation
This commit fixes the Version.fromString logic to identify legacy versions. It also adds an optional "distribution" field to the MainRespose for OpenSearch version 1.0.0+. Any preceeding versions that do not contain the distribution label will be handeled as legacy versions appropriately. Signed-off-by: Nicholas Walter Knize <[email protected]>
nknize
added
Re-version
|
✅ Gradle Wrapper Validation success c52f74f |
|
start gradle check |
|
✅ DCO Check Passed c52f74f |
|
✅ Gradle Precommit success c52f74f |
|
This is awesome Nick! |
| protected static final ImmutableOpenIntMap<Version> idToVersion; | ||
| protected static final ImmutableOpenMap<String, Version> stringToVersion; | ||
| static { |
There was a problem hiding this comment.
What's the reason behind moving the code to here?
I think we can refactor this to a separate class e.g. VersionCache or CachedVersions?
There was a problem hiding this comment.
classloader race condition. The static block requires fields of LegacyESVersion to be non-null but Version is loaded before LegacyESVersion since LegacyESVersion inherits from Version.
There was a problem hiding this comment.
Thanks for the explanation. PRed #608 that may have made this easier to catch.
| /** | ||
| * Returns the version given its string representation, current version if the argument is null or empty | ||
| */ | ||
| public static Version fromString(String version) { |
There was a problem hiding this comment.
Seems like this inherited from Version.java, duplicated?
There was a problem hiding this comment.
No.. it's a static method that needs to call the LegacyESVersion.fromStringSlow so it needs to be overloaded here. If inherited it will call the wrong implementation. (This is part of the hacky-ness that needs to be reworked over time).
|
Agree there's some more cleanup needed; and we probably want to add back support to |
Signed-off-by: Nicholas Walter Knize <[email protected]>
|
start gradle check |
|
✅ DCO Check Passed 2c37340 |
|
✅ Gradle Wrapper Validation success 2c37340 |
|
✅ Gradle Precommit success 2c37340 |
* Added "cjk" analyzer. Signed-off-by: pieper <[email protected]> * Adapted changelog. Signed-off-by: pieper <[email protected]> * Fixed license headers. Signed-off-by: pieper <[email protected]> * Tab vs. Spaces fix. Signed-off-by: pieper <[email protected]> --------- Signed-off-by: pieper <[email protected]>
Description
This PR fixes the Version.fromString logic to identify legacy versions. It
also adds an optional "distribution" field to the MainRespose for OpenSearch
version 1.0.0+. Any preceeding versions that do not contain the distribution
label will be handeled as legacy versions appropriately. This is tested in
remote reindex integration tests.
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.