-
Notifications
You must be signed in to change notification settings - Fork 25.7k
Replace TransportVersion.onOrAfter with TransportVersion.supports
#136760
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
Changes from 4 commits
d697cb1
39041d7
40705cd
7d4211d
f1be3c6
1f9c01d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -281,11 +281,27 @@ public static TransportVersion max(TransportVersion version1, TransportVersion v | |
| return version1.id > version2.id ? version1 : version2; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean onOrAfter(TransportVersion version) { | ||
| throw new UnsupportedOperationException("use TransportVersion.supports instead"); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean before(TransportVersion version) { | ||
| return version.id() > id(); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean between(TransportVersion lowerInclusive, TransportVersion upperExclusive) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Like the above, can this be blocked in a followup?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above. |
||
| if (upperExclusive.onOrBefore(lowerInclusive)) throw new IllegalArgumentException(); | ||
| return lowerInclusive.id <= id && before(upperExclusive); | ||
| } | ||
|
|
||
| /** | ||
| * Returns {@code true} if the specified version is compatible with this running version of Elasticsearch. | ||
| */ | ||
| public static boolean isCompatible(TransportVersion version) { | ||
| return version.onOrAfter(VersionsHolder.MINIMUM_COMPATIBLE); | ||
| return version.supports(VersionsHolder.MINIMUM_COMPATIBLE); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -376,7 +392,7 @@ public static TransportVersion fromString(String str) { | |
| * </ul> | ||
| */ | ||
| public boolean isPatchFrom(TransportVersion version) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like there are only a handful of remaining uses, perhaps these versions could be manually migrated (building a chained TransportVersion in memory) so that they can use supports and this could be made private?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a good idea. I will look into that. |
||
| return onOrAfter(version) && id < version.id + 100 - (version.id % 100); | ||
| return version.id <= id && id < version.id + 100 - (version.id % 100); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -417,7 +433,7 @@ public boolean isPatchFrom(TransportVersion version) { | |
| * } | ||
| */ | ||
| public boolean supports(TransportVersion version) { | ||
| if (onOrAfter(version)) { | ||
| if (version.id <= id) { | ||
| return true; | ||
| } | ||
| TransportVersion nextPatchVersion = version.nextPatchVersion; | ||
|
|
||
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 throw UOE here as well, suggesting
.supports(...) == false? Or is that intended for a followup?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.
That will 100% be a follow up. I just prefer to do one method at at time given how large this change is.