Migrate mapper-related modules to internal-*-rest-test #117298
Conversation
lkts
added
>non-issue
auto-backport
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
modules/reindex/build.gradle
Outdated
|
|
||
| clusterModules project(':modules:lang-painless') | ||
| clusterModules project(':modules:parent-join') | ||
| clusterModules(project(":modules:rest-root")) |
There was a problem hiding this comment.
nit: just for consistency you can omit the parens on this one
modules/reindex/build.gradle
Outdated
| // Whitelist reindexing from the local node so we can test reindex-from-remote. | ||
| setting 'reindex.remote.whitelist', '127.0.0.1:*' | ||
| requiresFeature 'es.index_mode_feature_flag_registered', Version.fromString("8.0.0") | ||
| if (buildParams.isSnapshotBuild() == false) { |
There was a problem hiding this comment.
Is this bit required? It wasn't in the old build script. Was this perhaps just copy/pasted from elsewhere?
There was a problem hiding this comment.
This is my attempt to replicate index_mode_feature_flag_registered that you asked about below. I realize now that it is a noop since it does not actually apply to a cluster. I have no clue what index_mode_feature_flag_registered does but it seems to not impact anything indeed. I'll research a bit.
There was a problem hiding this comment.
You are correct, you need to add the feature flag to the cluster. Feature flags are only required when running the tests in "release" mode. Maybe this flag isn't required anymore and is enabled by default in current versions so indeed may be unnecessary now. Probably worth confirming.
There was a problem hiding this comment.
Last usage of index_mode_feature_flag_registered was removed in #95220. It's a complete noop and should be removed everywhere.
There was a problem hiding this comment.
Awesome. This can be done separately in a follow up.
| module ':modules:rest-root' | ||
| // Whitelist reindexing from the local node so we can test reindex-from-remote. | ||
| setting 'reindex.remote.whitelist', '127.0.0.1:*' | ||
| requiresFeature 'es.index_mode_feature_flag_registered', Version.fromString("8.0.0") |
There was a problem hiding this comment.
Is this feature flag no longer required? I don't see it added to any of the migrated test classes.
| @@ -42,7 +42,3 @@ if (buildParams.isSnapshotBuild() == false) { | |||
| systemProperty 'es.index_mode_feature_flag_registered', 'true' | |||
| } | |||
| } | |||
|
|
|||
| testClusters.configureEach { | |||
| requiresFeature 'es.index_mode_feature_flag_registered', Version.fromString("8.0.0") | |||
There was a problem hiding this comment.
Same as before, do we not need this feature flag?
💔 Backport failed
You can use sqren/backport to manually backport by running |
(cherry picked from commit 2b8e4e7) # Conflicts: # modules/mapper-extras/build.gradle # plugins/mapper-annotated-text/build.gradle # plugins/mapper-murmur3/build.gradle # x-pack/plugin/mapper-unsigned-long/build.gradle # x-pack/plugin/mapper-version/build.gradle # x-pack/plugin/wildcard/build.gradle
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
(cherry picked from commit 2b8e4e7) # Conflicts: # modules/mapper-extras/build.gradle # plugins/mapper-annotated-text/build.gradle # plugins/mapper-murmur3/build.gradle # x-pack/plugin/mapper-unsigned-long/build.gradle # x-pack/plugin/mapper-version/build.gradle # x-pack/plugin/wildcard/build.gradle
…7407) (cherry picked from commit 2b8e4e7) # Conflicts: # modules/mapper-extras/build.gradle # plugins/mapper-annotated-text/build.gradle # plugins/mapper-murmur3/build.gradle # x-pack/plugin/mapper-unsigned-long/build.gradle # x-pack/plugin/mapper-version/build.gradle # x-pack/plugin/wildcard/build.gradle
…7406) (cherry picked from commit 2b8e4e7) # Conflicts: # modules/mapper-extras/build.gradle # plugins/mapper-annotated-text/build.gradle # plugins/mapper-murmur3/build.gradle # x-pack/plugin/mapper-unsigned-long/build.gradle # x-pack/plugin/mapper-version/build.gradle # x-pack/plugin/wildcard/build.gradle
Legacy yaml test runner does not support supplying test cluster with test features. Due to recently introduced
mapper.source.mode_from_index_settingtest feature, a lot of tests using it were silently ignored. This PR fixes this by migrating all projects using legacy runner andmapper.source.mode_from_index_settingto a "proper" test plugin.Related to #117284.
Related to #116072.