[ML][Data Frames] unify validation exceptions between PUT/_preview (#44983) (#45012)

* [ML][Data Frames] unify validation exceptions between PUT/_preview

* addressing PR comments
This commit is contained in:
Benjamin Trent 2019-07-30 13:05:07 -05:00 committed by GitHub
parent 979d0a71c7
commit 3f48720d41
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 76 additions and 11 deletions

View File

@ -104,6 +104,19 @@ public class TransportPreviewDataFrameTransformAction extends
} }
Pivot pivot = new Pivot(config.getPivotConfig()); Pivot pivot = new Pivot(config.getPivotConfig());
try {
pivot.validateConfig();
} catch (ElasticsearchStatusException e) {
listener.onFailure(
new ElasticsearchStatusException(DataFrameMessages.REST_PUT_DATA_FRAME_FAILED_TO_VALIDATE_DATA_FRAME_CONFIGURATION,
e.status(),
e));
return;
} catch (Exception e) {
listener.onFailure(new ElasticsearchStatusException(
DataFrameMessages.REST_PUT_DATA_FRAME_FAILED_TO_VALIDATE_DATA_FRAME_CONFIGURATION, RestStatus.INTERNAL_SERVER_ERROR, e));
return;
}
getPreview(pivot, config.getSource(), config.getDestination().getPipeline(), config.getDestination().getIndex(), listener); getPreview(pivot, config.getSource(), config.getDestination().getPipeline(), config.getDestination().getIndex(), listener);
} }

View File

@ -27,6 +27,7 @@ import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.license.LicenseUtils; import org.elasticsearch.license.LicenseUtils;
import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.persistent.PersistentTasksCustomMetaData; import org.elasticsearch.persistent.PersistentTasksCustomMetaData;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TransportService; import org.elasticsearch.transport.TransportService;
import org.elasticsearch.xpack.core.ClientHelper; import org.elasticsearch.xpack.core.ClientHelper;
@ -212,17 +213,32 @@ public class TransportPutDataFrameTransformAction extends TransportMasterNodeAct
// <2> Put our transform // <2> Put our transform
ActionListener<Boolean> pivotValidationListener = ActionListener.wrap( ActionListener<Boolean> pivotValidationListener = ActionListener.wrap(
validationResult -> dataFrameTransformsConfigManager.putTransformConfiguration(config, putTransformConfigurationListener), validationResult -> dataFrameTransformsConfigManager.putTransformConfiguration(config, putTransformConfigurationListener),
validationException -> listener.onFailure( validationException -> {
new RuntimeException(DataFrameMessages.REST_PUT_DATA_FRAME_FAILED_TO_VALIDATE_DATA_FRAME_CONFIGURATION, if (validationException instanceof ElasticsearchStatusException) {
validationException)) listener.onFailure(new ElasticsearchStatusException(
DataFrameMessages.REST_PUT_DATA_FRAME_FAILED_TO_VALIDATE_DATA_FRAME_CONFIGURATION,
((ElasticsearchStatusException)validationException).status(),
validationException));
} else {
listener.onFailure(new ElasticsearchStatusException(
DataFrameMessages.REST_PUT_DATA_FRAME_FAILED_TO_VALIDATE_DATA_FRAME_CONFIGURATION,
RestStatus.INTERNAL_SERVER_ERROR,
validationException));
}
}
); );
try { try {
pivot.validateConfig(); pivot.validateConfig();
} catch (ElasticsearchStatusException e) {
listener.onFailure(new ElasticsearchStatusException(
DataFrameMessages.REST_PUT_DATA_FRAME_FAILED_TO_VALIDATE_DATA_FRAME_CONFIGURATION,
e.status(),
e));
return;
} catch (Exception e) { } catch (Exception e) {
listener.onFailure( listener.onFailure(new ElasticsearchStatusException(
new RuntimeException(DataFrameMessages.REST_PUT_DATA_FRAME_FAILED_TO_VALIDATE_DATA_FRAME_CONFIGURATION, DataFrameMessages.REST_PUT_DATA_FRAME_FAILED_TO_VALIDATE_DATA_FRAME_CONFIGURATION, RestStatus.INTERNAL_SERVER_ERROR, e));
e));
return; return;
} }

View File

@ -8,6 +8,7 @@ package org.elasticsearch.xpack.dataframe.transforms.pivot;
import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.Logger;
import org.elasticsearch.ElasticsearchStatusException;
import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.search.SearchAction; import org.elasticsearch.action.search.SearchAction;
import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.search.SearchRequest;
@ -72,7 +73,7 @@ public class Pivot {
public void validateConfig() { public void validateConfig() {
for (AggregationBuilder agg : config.getAggregationConfig().getAggregatorFactories()) { for (AggregationBuilder agg : config.getAggregationConfig().getAggregatorFactories()) {
if (Aggregations.isSupportedByDataframe(agg.getType()) == false) { if (Aggregations.isSupportedByDataframe(agg.getType()) == false) {
throw new RuntimeException("Unsupported aggregation type [" + agg.getType() + "]"); throw new ElasticsearchStatusException("Unsupported aggregation type [" + agg.getType() + "]", RestStatus.BAD_REQUEST);
} }
} }
} }
@ -82,15 +83,17 @@ public class Pivot {
client.execute(SearchAction.INSTANCE, searchRequest, ActionListener.wrap(response -> { client.execute(SearchAction.INSTANCE, searchRequest, ActionListener.wrap(response -> {
if (response == null) { if (response == null) {
listener.onFailure(new RuntimeException("Unexpected null response from test query")); listener.onFailure(new ElasticsearchStatusException("Unexpected null response from test query",
RestStatus.SERVICE_UNAVAILABLE));
return; return;
} }
if (response.status() != RestStatus.OK) { if (response.status() != RestStatus.OK) {
listener.onFailure(new RuntimeException("Unexpected status from response of test query: "+ response.status())); listener.onFailure(new ElasticsearchStatusException("Unexpected status from response of test query: " + response.status(),
response.status()));
return; return;
} }
listener.onResponse(true); listener.onResponse(true);
}, e -> listener.onFailure(new RuntimeException("Failed to test query", e)))); }, e -> listener.onFailure(new ElasticsearchStatusException("Failed to test query", RestStatus.SERVICE_UNAVAILABLE, e))));
} }
public void deduceMappings(Client client, SourceConfig sourceConfig, final ActionListener<Map<String, String>> listener) { public void deduceMappings(Client client, SourceConfig sourceConfig, final ActionListener<Map<String, String>> listener) {

View File

@ -7,6 +7,7 @@
package org.elasticsearch.xpack.dataframe.transforms.pivot; package org.elasticsearch.xpack.dataframe.transforms.pivot;
import org.apache.lucene.search.TotalHits; import org.apache.lucene.search.TotalHits;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.action.ActionType; import org.elasticsearch.action.ActionType;
import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionRequest;
@ -144,7 +145,7 @@ public class PivotTests extends ESTestCase {
AggregationConfig aggregationConfig = getAggregationConfig(agg); AggregationConfig aggregationConfig = getAggregationConfig(agg);
Pivot pivot = new Pivot(getValidPivotConfig(aggregationConfig)); Pivot pivot = new Pivot(getValidPivotConfig(aggregationConfig));
RuntimeException ex = expectThrows(RuntimeException.class, pivot::validateConfig); ElasticsearchException ex = expectThrows(ElasticsearchException.class, pivot::validateConfig);
assertThat("expected aggregations to be unsupported, but they were", ex, is(notNullValue())); assertThat("expected aggregations to be unsupported, but they were", ex, is(notNullValue()));
} }
} }

View File

@ -252,3 +252,35 @@ setup:
} }
} }
} }
---
"Test preview with unsupported agg":
- do:
catch: bad_request
data_frame.preview_data_frame_transform:
body: >
{
"source": { "index": "airline-data" },
"dest": { "pipeline": "missing-pipeline" },
"pivot": {
"group_by": {
"time": {"date_histogram": {"fixed_interval": "1h", "field": "time"}}},
"aggs": {
"vals": {"terms": {"field":"airline"}}
}
}
}
- do:
catch: /Unsupported aggregation type \[terms\]/
data_frame.preview_data_frame_transform:
body: >
{
"source": { "index": "airline-data" },
"dest": { "pipeline": "missing-pipeline" },
"pivot": {
"group_by": {
"time": {"date_histogram": {"fixed_interval": "1h", "field": "time"}}},
"aggs": {
"vals": {"terms": {"field":"airline"}}
}
}
}