more consistent allow_no_indices=false errors#142373
more consistent allow_no_indices=false errors#142373richard-dennehy merged 11 commits intoelastic:mainfrom
Conversation
| remote | ||
| ); | ||
| assertNotNull(ex); | ||
| assertThat(ex.getMessage(), equalTo("no such index []")); |
There was a problem hiding this comment.
Do we want a better error message than this? I'm not sure what it could say, though 🤔
There was a problem hiding this comment.
I think we can report whatever the original expresssions are, e.g. no such index [-logs*], etc. This is not very different from resolution for something like no-match-*.
| if (localResolvedExpressions.localIndicesIsEmpty() | ||
| && remoteResolvedExpressions.values().stream().allMatch(ResolvedIndexExpressions::localIndicesIsEmpty)) { | ||
| return new IndexNotFoundException(""); | ||
| } |
There was a problem hiding this comment.
I'm not delighted with what's effectively "loop over the entire list of local expressions and also loop over all the remote expressions again", but it seems difficult to incorporate this logic into the existing loop.
Tbh I'm starting to get concerned that validate is becoming too complicated to reason about - I'm having a very hard time following the logic without using a debugger
There was a problem hiding this comment.
Yeah I raised ES-13804 a while back to improve the validator. The most relevant is the 2nd point from the Jira issue, which suggests extracting the logic into a per-validation class. I think it's a good candidate to work on after fixing the existing two bugs.
|
Pinging @elastic/es-security (Team:Security) |
| if (notFoundException == null && indicesOptions.allowNoIndices() == false) { | ||
| if (localResolvedExpressions.localIndicesIsEmpty() | ||
| && remoteResolvedExpressions.values().stream().allMatch(ResolvedIndexExpressions::localIndicesIsEmpty)) { | ||
| return new IndexNotFoundException(localResolvedExpressions.expressions().getFirst().original()); |
There was a problem hiding this comment.
I think we may want to concate all local original expressions to a single string for the report, similar to how it is done here in IndicesAndAliasesResolver.
…on-sliced-reindex
* upstream/main:
Mute org.elasticsearch.reindex.management.ReindexManagementClientYamlTestSuiteIT test {yaml=reindex/30_cancel_reindex/Cancel running reindex returns response and GET confirms completed} elastic#142079
[ESQL] Fix async logging consistency and severity (elastic#142401)
more consistent allow_no_indices=false errors (elastic#142373)
Attribute ES|QL shard search load in Lucene operators (elastic#142841)
MetricsInfoOperator refactoring (elastic#142935)
Resolving certain index expressions returns an empty list of indices, even when
allow_no_indices=false; this PR adds extra validation to fix some of these, e.g.-logs*,concrete-index-1,-concrete-index-1