-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Remove the 6.7 version constants. #42039
Remove the 6.7 version constants. #42039
Conversation
Pinging @elastic/es-core-infra |
e83b021
to
ba3f5ba
Compare
ba3f5ba
to
83da48c
Compare
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.
LGTM, just left two comments around random version testing
@@ -886,7 +886,7 @@ public void testShardLockObtainFailedException() throws IOException { | |||
|
|||
public void testSnapshotInProgressException() throws IOException { | |||
SnapshotInProgressException orig = new SnapshotInProgressException("boom"); | |||
Version version = VersionUtils.randomVersionBetween(random(), Version.V_6_7_0, Version.CURRENT); | |||
Version version = VersionUtils.randomVersionBetween(random(), Version.V_7_0_0, Version.CURRENT); |
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.
no need to do here, but we could probably replace this so that we do not need to worry about it in the big 7.x refactor.
to something like
Version version = Versionutils.randomVersion()
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.
I think it should be VersionUtils.randomIndexCompatibleVersion(random())
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.
👍 I'll make this change in the current PR.
@@ -143,7 +143,7 @@ public void setupClient() { | |||
// tokens docs on a separate index), let's test the TokenService works in a mixed cluster with nodes with versions prior to these | |||
// developments | |||
if (randomBoolean()) { | |||
oldNode = addAnotherDataNodeWithVersion(this.clusterService, randomFrom(Version.V_6_7_0, Version.V_7_0_0)); | |||
oldNode = addAnotherDataNodeWithVersion(this.clusterService, Version.V_7_0_0); |
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.
should this be a range like so?
oldNode = addAnotherDataNodeWithVersion(this.clusterService, VersionUtils.randomVersionBetween(random(), Version.V_7_0_0, Version.V_7_1_0));
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.
Makes sense given the comment above, will update this too.
@elasticmachine run elasticsearch-ci/packaging-sample |
@elasticmachine run elasticsearch-ci/1 |
This PR removes all constants of the form `Version.V_6_7_*`, since master no longer needs to account for them. Relates to elastic#41164.
This PR removes all constants of the form
Version.V_6_7_*
, since master nolonger needs to account for them.
Relates to #41164.