honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[ML] Handle failures in the idiomatic way (elastic/x-pack-elasticsearch#1785) 

This commit changes a couple of places where our ExceptionsHelper
class was throwing exceptions to instead return the exceptions.
Then they can be passed to onFailure() methods or thrown depending
on what's appropriate for the caller.  This is the standard Elastic
way of handling failures.

Original commit: elastic/x-pack-elasticsearch@fce07eb075
This commit is contained in:
David Roberts 2017-06-20 13:06:28 +01:00 committed by GitHub
parent d6254c9fd3
commit 96927cc1b6
3 changed files with 13 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
plugin/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java
View File

@ -167,6 +167,7 @@ public class JobManager extends AbstractComponent {
MlMetadata currentMlMetadata = state.metaData().custom(MlMetadata.TYPE); MlMetadata currentMlMetadata = state.metaData().custom(MlMetadata.TYPE);
if (currentMlMetadata != null && currentMlMetadata.getJobs().containsKey(job.getId())) { if (currentMlMetadata != null && currentMlMetadata.getJobs().containsKey(job.getId())) {
actionListener.onFailure(ExceptionsHelper.jobAlreadyExists(job.getId())); actionListener.onFailure(ExceptionsHelper.jobAlreadyExists(job.getId()));
return;
} }
ActionListener<Boolean> putJobListener = new ActionListener<Boolean>() { ActionListener<Boolean> putJobListener = new ActionListener<Boolean>() {

4
plugin/src/main/java/org/elasticsearch/xpack/ml/utils/ExceptionsHelper.java
View File

@ -23,11 +23,11 @@ public class ExceptionsHelper {
} }
public static ResourceAlreadyExistsException jobAlreadyExists(String jobId) { public static ResourceAlreadyExistsException jobAlreadyExists(String jobId) {
throw new ResourceAlreadyExistsException(Messages.getMessage(Messages.JOB_CONFIG_ID_ALREADY_TAKEN, jobId)); return new ResourceAlreadyExistsException(Messages.getMessage(Messages.JOB_CONFIG_ID_ALREADY_TAKEN, jobId));
} }
public static ResourceNotFoundException missingDatafeedException(String datafeedId) { public static ResourceNotFoundException missingDatafeedException(String datafeedId) {
throw new ResourceNotFoundException(Messages.getMessage(Messages.DATAFEED_NOT_FOUND, datafeedId)); return new ResourceNotFoundException(Messages.getMessage(Messages.DATAFEED_NOT_FOUND, datafeedId));
} }
public static ElasticsearchException serverError(String msg) { public static ElasticsearchException serverError(String msg) {

21
plugin/src/test/java/org/elasticsearch/xpack/ml/job/JobManagerTests.java
View File

@ -167,18 +167,17 @@ public class JobManagerTests extends ESTestCase {
ClusterState clusterState = ClusterState.builder(new ClusterName("name")) ClusterState clusterState = ClusterState.builder(new ClusterName("name"))
.metaData(MetaData.builder().putCustom(MlMetadata.TYPE, mlMetadata.build())).build(); .metaData(MetaData.builder().putCustom(MlMetadata.TYPE, mlMetadata.build())).build();
expectThrows(ResourceAlreadyExistsException.class, () -> jobManager.putJob(putJobRequest, clusterState, new ActionListener<PutJobAction.Response>() {
jobManager.putJob(putJobRequest, clusterState, new ActionListener<PutJobAction.Response>() { @Override
@Override public void onResponse(PutJobAction.Response response) {
public void onResponse(PutJobAction.Response response) { fail("should have got an error");
}
} @Override
public void onFailure(Exception e) {
@Override assertTrue(e instanceof ResourceAlreadyExistsException);
public void onFailure(Exception e) { }
fail(e.toString()); });
}
}));
} }
private Job.Builder createJob() { private Job.Builder createJob() {