Skip to content

Commit 22b36ea

Browse files
[ML] Remove error on parsing progress for unknown phase in DFA (#55926)
On second thought, this check does not seem to be adding value. We can test that the phases are as we expect them for each analysis by adding yaml tests. Those would fail if we introduce new phases from c++ accidentally or without coordination. This would achieve the same thing. At the same time we would not have to comment out this code each time a new phase is introduced. Instead we can just temporarily mute those yaml tests. Note I will add those tests right after the imminent new phases are added to the c++ side.
1 parent 5ef7aac commit 22b36ea

File tree

2 files changed

+6
-9
lines changed

2 files changed

+6
-9
lines changed

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/dataframe/stats/ProgressTracker.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
*/
66
package org.elasticsearch.xpack.ml.dataframe.stats;
77

8-
import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper;
98
import org.elasticsearch.xpack.core.ml.utils.PhaseProgress;
109

1110
import java.util.ArrayList;
@@ -74,10 +73,7 @@ public int getWritingResultsProgressPercent() {
7473
}
7574

7675
public void updatePhase(PhaseProgress phase) {
77-
Integer newValue = progressPercentPerPhase.computeIfPresent(phase.getPhase(), (k, v) -> phase.getProgressPercent());
78-
if (newValue == null) {
79-
throw ExceptionsHelper.serverError("unknown progress phase [" + phase.getPhase() + "]");
80-
}
76+
progressPercentPerPhase.computeIfPresent(phase.getPhase(), (k, v) -> phase.getProgressPercent());
8177
}
8278

8379
public List<PhaseProgress> report() {

x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/dataframe/stats/ProgressTrackerTests.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
package org.elasticsearch.xpack.ml.dataframe.stats;
88

9-
import org.elasticsearch.ElasticsearchException;
109
import org.elasticsearch.test.ESTestCase;
1110
import org.elasticsearch.xpack.core.ml.utils.PhaseProgress;
1211

@@ -73,9 +72,11 @@ public void testUpdates() {
7372
public void testUpdatePhase_GivenUnknownPhase() {
7473
ProgressTracker progressTracker = ProgressTracker.fromZeroes(Collections.singletonList("foo"));
7574

76-
ElasticsearchException e = expectThrows(ElasticsearchException.class,
77-
() -> progressTracker.updatePhase(new PhaseProgress("bar", 42)));
75+
progressTracker.updatePhase(new PhaseProgress("unknown", 42));
76+
List<PhaseProgress> phases = progressTracker.report();
7877

79-
assertThat(e.getMessage(), equalTo("unknown progress phase [bar]"));
78+
assertThat(phases.size(), equalTo(4));
79+
assertThat(phases.stream().map(PhaseProgress::getPhase).collect(Collectors.toList()),
80+
contains("reindexing", "loading_data", "foo", "writing_results"));
8081
}
8182
}

0 commit comments

Comments
 (0)