[ML] Skip dataframes when disabled #137220
Conversation
When Dataframes are disabled, do not try to call the Action and instead assume there are no trained models associated with a dataframe job. This avoids a transport exception for the disabled action.
|
Hi @prwhelan, I've created a changelog YAML for you. |
elasticsearchmachine
added
the
serverless-linked
|
Pinging @elastic/ml-core (Team:ML) |
| } | ||
|
|
||
| public void testBuildTableAccumulatedStats() { | ||
| var action = new RestCatTrainedModelsAction(true); |
There was a problem hiding this comment.
Would it be possible to add a unit test for the case where areDataFrameAnalyticsEnabled is false and we don't expect to see the dataframe information in the table?
There was a problem hiding this comment.
Sure, but the existing functionality isn't tested so adding the tests for both true/false would take a few weeks of calendar time, which I assume is okay since this bug had been open for months. Alternatively, the linked integration test covers the false case whereas the existing integration tests cover the true case.
There was a problem hiding this comment.
Can you not just basically copy this test case where areDataFrameAnalyticsEnabled is true, switch it to false, then assert that we see __none__ as the data_frame.id value?
There was a problem hiding this comment.
That boolean is used within getDerivedData which is called before the method that this code tests, which is buildTable. The test only calls buildTable, so we'd only be verifying that an absent list sets __none__ which is a test we should have but isn't really relevant to this change (happy to add it, though)
There was a problem hiding this comment.
Aah, I see, thanks for clarifying. Yeah, no need to add that test in this PR, since it's basically unrelated.
|
@elasticmachine update branch please |
|
There are no new commits on the base branch. |
4 participants
When Dataframes are disabled, do not try to call the Action and instead assume there are no trained models associated with a dataframe job. This avoids a transport exception for the disabled action.
This only affects serverless, so we do not need to backport this to previous versions.