Move resolve aliases to IndexAbstractionOptions#143953
Move resolve aliases to IndexAbstractionOptions#143953jfreden wants to merge 4 commits intoelastic:mainfrom
Conversation
There has been several discussions on this topic since I opened #141050 and @idegtiarenko also mentioned this in #143561 (comment). The issue is that both `resolveViews` and `resolveAliases` are in `WildcardOptions` but are used when resolving both concrete index pattern targets and wildcard targets. This PR moves `resolveViews` from `IndicesOptions.WildcardOptions` into a new `IndicesOptions.IndexAbstractionOptions` record to address this confusing inconsistency. This is in preparation for sending `resolveViews` as a parameter to field caps for remote view resolving where more work will be done to serialize `resolveViews` and not having this in place would make for confusing code. See #143384 for more information on the upcoming serialization of `resolveViews`. **_Note_**: `resolveAliases` is moved in a follow up PR since it's decoupled from the resolveViews changes. #143953 **_Note_**: `IndicesRequest.includeDataStreams()` is the same type of flag as `resolveAliases` and `resolveViews`, it controls whether data streams participate in index resolution. It's a candidate for `IndexAbstractionOptions` but currently lives on `IndicesRequest` (not `IndicesOptions`) and is threaded separately through `IndexNameExpressionResolver.Context` as a constructor parameter. Moving it would touch 60+ files and change the `IndicesRequest` interface. Because of how big that change would be, I have not considered doing that in this PR (or at all).
6010006 to
015e98e
Compare
30caf92 to
1402f33
Compare
|
Pinging @elastic/es-security (Team:Security) |
|
|
||
| public static Builder builder(IndexAbstractionOptions indexAbstractionOptions) { | ||
| return new Builder(indexAbstractionOptions); | ||
| } |
There was a problem hiding this comment.
NIT: we have 2 static builder methods today: one for starting from scratch and one to start editing other abstraction. I wonder if we should change it to be instance method:
IndicesOptions.builder(options.indexAbstractionOptions())...build() -> options.indexAbstractionOptions().builder()...build().
It is different from existing approach but it looks a bit cleaner to me.
There was a problem hiding this comment.
I agree that is a lot nicer, but the other IndicesOptions are using that style of builder so I think it would be inconsistent and require a full refactor of all of them, which I think is out of scope for this PR.
There was a problem hiding this comment.
I agree, this is not blocking this PR
This is a follow up from #143741 to also move resolve aliases to
IndexAbstractionOptions.