Fix issues with ignore_missing_component_templates #95527
Conversation
eyalkoren added >bug Team:Data Management 
elasticsearchmachine added external-contributor elasticsearchmachine removed the Team:Data Management eyalkoren added the :Data Management/Data streams elasticsearchmachine added the Team:Data Management 
| Hi @eyalkoren, I've created a changelog YAML for you. |
elasticsearchmachine removed the needs:triage | Pinging @elastic/es-data-management (Team:Data Management) |
Is this really the case? I thought I had tested this and should also be covered by this test case? https://github.com/elastic/elasticsearch/pull/92436/files#diff-b5daa47f1a90ec68657df8ffb1bc5bd5c65561c66be53edd2088d6f79767d1a6R307 |
| public class TestUtils { | ||
| public static <T> T assetNoException(Callable<T> callable, @Nullable String msg) { | ||
| try { | ||
| return callable.call(); | ||
| } catch (Throwable th) { | ||
| throw new AssertionError(((msg != null) ? msg + "; original error: " : "unexpected error: ") + th.getMessage()); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Any exception thrown will fail the test, rather than having a construct explicitly catching exceptions and causing an assertion. We don't need to add this for now (we try to be very conservative with utility classes since we have so many.)
There was a problem hiding this comment.
There are benefits with proper assertions, rather than letting a test just fail. Mainly, the ability to print a more indicative message on what may be the actual regression leading to future failures. In addition, anything that expects assertion failures to be the same would benefit from that. For example, in IntelliJ an assertion failure will have a different icon from any other uncaught exception, which I think is preferable for troubleshooting. This was a very simple attempt to offer a functionality similar to assertDoesNotThrow() that was introduced in JUnit 5.
we try to be very conservative with utility classes since we have so many
No problem, removing
| @gmarouli I'm re-adding the |
@ruflin right, I saw this test and should have been more specific- such failures occur when an inherent composable template gets added to the registry through the automatic application of such, based on cluster state. See the I changed the original PR description accordingly. |
…into fix-missing-template-issues
💔 Backport failed
You can use sqren/backport to manually backport by running |
(cherry picked from commit 37d2da1)
(cherry picked from commit 37d2da1)
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
6 participants
A fix for two issues that were introduced with the addition of
ignore_missing_component_templates:ignore_missing_component_templates. Such templates can be removed, as they are not required for creation of new indices.ignore_missing_component_templates. This is in contradiction to manually registering a composable template with missing non-required component templates, as demonstrated in an existing test case.