[7.x][ML] Remove error on parsing progress for unknown phase in DFA (#55926) (#55954)

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.

Backport of #55926
This commit is contained in:
Dimitris Athanasiou 2020-04-29 20:11:33 +03:00 committed by GitHub
parent 9eb6736500
commit c5aa281171
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 6 additions and 9 deletions

View File

@ -5,7 +5,6 @@
*/
package org.elasticsearch.xpack.ml.dataframe.stats;
import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper;
import org.elasticsearch.xpack.core.ml.utils.PhaseProgress;
import java.util.ArrayList;
@ -75,10 +74,7 @@ public class ProgressTracker {
}
public void updatePhase(PhaseProgress phase) {
Integer newValue = progressPercentPerPhase.computeIfPresent(phase.getPhase(), (k, v) -> phase.getProgressPercent());
if (newValue == null) {
throw ExceptionsHelper.serverError("unknown progress phase [" + phase.getPhase() + "]");
}
progressPercentPerPhase.computeIfPresent(phase.getPhase(), (k, v) -> phase.getProgressPercent());
}
public List<PhaseProgress> report() {

View File

@ -6,7 +6,6 @@
package org.elasticsearch.xpack.ml.dataframe.stats;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.core.ml.utils.PhaseProgress;
@ -73,9 +72,11 @@ public class ProgressTrackerTests extends ESTestCase {
public void testUpdatePhase_GivenUnknownPhase() {
ProgressTracker progressTracker = ProgressTracker.fromZeroes(Collections.singletonList("foo"));
ElasticsearchException e = expectThrows(ElasticsearchException.class,
() -> progressTracker.updatePhase(new PhaseProgress("bar", 42)));
progressTracker.updatePhase(new PhaseProgress("unknown", 42));
List<PhaseProgress> phases = progressTracker.report();
assertThat(e.getMessage(), equalTo("unknown progress phase [bar]"));
assertThat(phases.size(), equalTo(4));
assertThat(phases.stream().map(PhaseProgress::getPhase).collect(Collectors.toList()),
contains("reindexing", "loading_data", "foo", "writing_results"));
}
}