-
Notifications
You must be signed in to change notification settings - Fork 25.3k
Remove Version 6 pre-release constants #41517
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
Conversation
Pinging @elastic/es-core-infra |
4e11423
to
352ecd4
Compare
352ecd4
to
c311574
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.
Thanks for helping out here! I left a handful of comments. The major issue is we shouldn't really have any new uses of the v7.0.0 constant. Instead, those test uses should be converted to using an appropriate version through VersionUtils, or using Version.CURRENT.minimumIndexCompatibleVersion()
.
server/src/main/java/org/elasticsearch/index/mapper/DynamicTemplate.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/index/translog/Translog.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/action/admin/indices/create/SplitIndexIT.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/cluster/routing/allocation/FailedNodeRoutingTests.java
Outdated
Show resolved
Hide resolved
Thanks @rjernst, I pushed updates that should adress your comments. |
@rjernst this would be ready for another look if you have the time |
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.
Looks better, but there are still some hardcoded constant usages.
server/src/test/java/org/elasticsearch/cluster/routing/allocation/FailedShardsRoutingTests.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplateTests.java
Outdated
Show resolved
Hide resolved
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/MlMetadata.java
Outdated
Show resolved
Hide resolved
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/TokenMetaData.java
Outdated
Show resolved
Hide resolved
...ava/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsMonitoringDocTests.java
Outdated
Show resolved
Hide resolved
...va/org/elasticsearch/xpack/monitoring/collector/indices/IndexRecoveryMonitoringDocTests.java
Outdated
Show resolved
Hide resolved
@rjernst thanks for the update, I pushed another commit that should adress your last comments. |
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.
Thanks @cbuescher. There are a just a few remaining places I see using the hardcoded version constants.
server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeServiceTests.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/cluster/routing/allocation/FailedShardsRoutingTests.java
Outdated
Show resolved
Hide resolved
test/framework/src/test/java/org/elasticsearch/test/VersionUtilsTests.java
Outdated
Show resolved
Hide resolved
...ava/org/elasticsearch/xpack/monitoring/collector/cluster/ClusterStatsMonitoringDocTests.java
Outdated
Show resolved
Hide resolved
d406d64
to
7b84d64
Compare
@rjernst looks ready for another round. |
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.
Thanks @cbuescher! LGTM
…rsing * elastic/master: [ML] relax set upgrade mode test to match what is guaranteed (elastic#41958) Add note about ILM action ordering (elastic#41771) Remove Version 6 pre-release constants (elastic#41517) Mute illegal interval rollup tests Add static section whitelist info to api docs generation (elastic#41870) Cleanup RollupSearch exceptions, disallow partial results (elastic#41272)
Now that master is 8.0, we can remove uses of these constants and the backcompat code that uses them, since 8 will always walk to 7.0+ nodes. This PR starts by removing the pre-6 release constants, remove obsolete code and replacing its occurances in tests where needed. Relates to elastic#41164
Now that master is 8.0, we can remove uses of these constants and the
backcompat code that uses them, since 8 will always walk to 7.0+ nodes.
This PR starts by removing the pre-6 release constants, remove obsolete code
and replacing its occurances in tests where needed.
Relates to #41164