REST high-level client: add support for Indices Update Settings API [take 2] #29327
Conversation
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
1 similar comment
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
|
Pinging @elastic/es-core-infra |
|
hi @olcbean thanks for your effort, of trying to address the issue caused in x-pack tests that you can't even look at :) I didn't see this coming unfortunately. The problem in your previous PR, and partially in this updated one too, is that we change equals/hashcode implementation for base classes that are subclassed by a lot of classes. At that point we should also update all those classes otherwise e.g. two different cluster health requests are deemed equal as we only use fields from its base class to do the comparison. Rather than making this PR huge though I think I would only modify equals/hashcode for the request that we are working on without affecting its base classes hence all of the sublcasses of such base classes. One day, or in a follow-up PR, we can do this properly, but for now I think that makes the change too big and unnecessary. How does this sound to you? |
|
@javanna it is definitely better to address the equals / hashCode gradually rather than as part of this PR ;) |
|
btw is this the first Request for which |
Yes it may be the first one, most of the request tests are probably just |
| return Objects.equals(settings, that.settings) | ||
| && Objects.equals(indicesOptions, that.indicesOptions) | ||
| && Objects.equals(preserveExisting, that.preserveExisting) | ||
| && Arrays.equals(indices, that.indices); |
There was a problem hiding this comment.
don't you need to take also timeout and masterNodeTimeout into account?
There was a problem hiding this comment.
I would suspect proper equals/hashcode tests to fail currently, maybe also tests need to be updated to mutate such fields.
There was a problem hiding this comment.
Now the equals and the hashCode use all request fields except for the flatSettings ( as the flatSettings are not serialized ).
|
test this please |
|
retest this please |
|
retest this please |
1 similar comment
|
retest this please |
|
retest this please |
|
Thanks @olcbean once again and sorry it took me a while to merge it. |
* master: (96 commits) TEST: Mute testEnsureWeReconnect Mute full cluster restart test recovery test REST high-level client: add support for Indices Update Settings API [take 2] (elastic#29327) Plugins: Fix native controller confirmation for non-meta plugin (elastic#29434) Remove PipelineExecutionService#executeIndexRequest (elastic#29537) Fix overflow error in parsing of long geohashes (elastic#29418) Remove unused index.ttl.disable_purge setting (elastic#29527) FullClusterRestartIT.testRecovery should wait for all initializing shards Build: Fail if any libs depend on non-core libs (elastic#29336) [TEST] REST client request without leading '/' (elastic#29471) Using ObjectParser in UpdateRequest (elastic#29293) Prevent accidental changes of default values (elastic#29528) [Docs] Add definitions to glossary (elastic#29127) Avoid self-deadlock in the translog (elastic#29520) Minor cleanup in NodeInfo.groovy Lazy configure build tasks that require older JDKs (elastic#29519) Simplify snapshot check in root build file Make NodeInfo#nodeVersion strongly-typed as Version (elastic#29515) Enable license header exclusions (elastic#29379) Use proper Java version for BWC builds (elastic#29493) ...
Relates to #27205