Skip to content

Commit 3f48720

Browse files
benwtrentbenwtrent
authored
[ML][Data Frames] unify validation exceptions between PUT/_preview (#44983) (#45012)
* [ML][Data Frames] unify validation exceptions between PUT/_preview * addressing PR comments
1 parent 979d0a7 commit 3f48720

File tree

5 files changed

+76
-11
lines changed

5 files changed

+76
-11
lines changed

‎x-pack/plugin/data-frame/src/main/java/org/elasticsearch/xpack/dataframe/action/TransportPreviewDataFrameTransformAction.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,19 @@ protected void doExecute(Task task,
104104
}
105105

106106
Pivot pivot = new Pivot(config.getPivotConfig());
107+
try {
108+
pivot.validateConfig();
109+
} catch (ElasticsearchStatusException e) {
110+
listener.onFailure(
111+
new ElasticsearchStatusException(DataFrameMessages.REST_PUT_DATA_FRAME_FAILED_TO_VALIDATE_DATA_FRAME_CONFIGURATION,
112+
e.status(),
113+
e));
114+
return;
115+
} catch (Exception e) {
116+
listener.onFailure(new ElasticsearchStatusException(
117+
DataFrameMessages.REST_PUT_DATA_FRAME_FAILED_TO_VALIDATE_DATA_FRAME_CONFIGURATION, RestStatus.INTERNAL_SERVER_ERROR, e));
118+
return;
119+
}
107120

108121
getPreview(pivot, config.getSource(), config.getDestination().getPipeline(), config.getDestination().getIndex(), listener);
109122
}

‎x-pack/plugin/data-frame/src/main/java/org/elasticsearch/xpack/dataframe/action/TransportPutDataFrameTransformAction.java

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.elasticsearch.license.LicenseUtils;
2828
import org.elasticsearch.license.XPackLicenseState;
2929
import org.elasticsearch.persistent.PersistentTasksCustomMetaData;
30+
import org.elasticsearch.rest.RestStatus;
3031
import org.elasticsearch.threadpool.ThreadPool;
3132
import org.elasticsearch.transport.TransportService;
3233
import org.elasticsearch.xpack.core.ClientHelper;
@@ -212,17 +213,32 @@ private void putDataFrame(Request request, ActionListener<AcknowledgedResponse>
212213
// <2> Put our transform
213214
ActionListener<Boolean> pivotValidationListener = ActionListener.wrap(
214215
validationResult -> dataFrameTransformsConfigManager.putTransformConfiguration(config, putTransformConfigurationListener),
215-
validationException -> listener.onFailure(
216-
new RuntimeException(DataFrameMessages.REST_PUT_DATA_FRAME_FAILED_TO_VALIDATE_DATA_FRAME_CONFIGURATION,
217-
validationException))
216+
validationException -> {
217+
if (validationException instanceof ElasticsearchStatusException) {
218+
listener.onFailure(new ElasticsearchStatusException(
219+
DataFrameMessages.REST_PUT_DATA_FRAME_FAILED_TO_VALIDATE_DATA_FRAME_CONFIGURATION,
220+
((ElasticsearchStatusException)validationException).status(),
221+
validationException));
222+
} else {
223+
listener.onFailure(new ElasticsearchStatusException(
224+
DataFrameMessages.REST_PUT_DATA_FRAME_FAILED_TO_VALIDATE_DATA_FRAME_CONFIGURATION,
225+
RestStatus.INTERNAL_SERVER_ERROR,
226+
validationException));
227+
}
228+
}
218229
);
219230

220231
try {
221232
pivot.validateConfig();
233+
} catch (ElasticsearchStatusException e) {
234+
listener.onFailure(new ElasticsearchStatusException(
235+
DataFrameMessages.REST_PUT_DATA_FRAME_FAILED_TO_VALIDATE_DATA_FRAME_CONFIGURATION,
236+
e.status(),
237+
e));
238+
return;
222239
} catch (Exception e) {
223-
listener.onFailure(
224-
new RuntimeException(DataFrameMessages.REST_PUT_DATA_FRAME_FAILED_TO_VALIDATE_DATA_FRAME_CONFIGURATION,
225-
e));
240+
listener.onFailure(new ElasticsearchStatusException(
241+
DataFrameMessages.REST_PUT_DATA_FRAME_FAILED_TO_VALIDATE_DATA_FRAME_CONFIGURATION, RestStatus.INTERNAL_SERVER_ERROR, e));
226242
return;
227243
}
228244

‎x-pack/plugin/data-frame/src/main/java/org/elasticsearch/xpack/dataframe/transforms/pivot/Pivot.java

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

99
import org.apache.logging.log4j.LogManager;
1010
import org.apache.logging.log4j.Logger;
11+
import org.elasticsearch.ElasticsearchStatusException;
1112
import org.elasticsearch.action.ActionListener;
1213
import org.elasticsearch.action.search.SearchAction;
1314
import org.elasticsearch.action.search.SearchRequest;
@@ -72,7 +73,7 @@ public Pivot(PivotConfig config) {
7273
public void validateConfig() {
7374
for (AggregationBuilder agg : config.getAggregationConfig().getAggregatorFactories()) {
7475
if (Aggregations.isSupportedByDataframe(agg.getType()) == false) {
75-
throw new RuntimeException("Unsupported aggregation type [" + agg.getType() + "]");
76+
throw new ElasticsearchStatusException("Unsupported aggregation type [" + agg.getType() + "]", RestStatus.BAD_REQUEST);
7677
}
7778
}
7879
}
@@ -82,15 +83,17 @@ public void validateQuery(Client client, SourceConfig sourceConfig, final Action
8283

8384
client.execute(SearchAction.INSTANCE, searchRequest, ActionListener.wrap(response -> {
8485
if (response == null) {
85-
listener.onFailure(new RuntimeException("Unexpected null response from test query"));
86+
listener.onFailure(new ElasticsearchStatusException("Unexpected null response from test query",
87+
RestStatus.SERVICE_UNAVAILABLE));
8688
return;
8789
}
8890
if (response.status() != RestStatus.OK) {
89-
listener.onFailure(new RuntimeException("Unexpected status from response of test query: "+ response.status()));
91+
listener.onFailure(new ElasticsearchStatusException("Unexpected status from response of test query: " + response.status(),
92+
response.status()));
9093
return;
9194
}
9295
listener.onResponse(true);
93-
}, e -> listener.onFailure(new RuntimeException("Failed to test query", e))));
96+
}, e -> listener.onFailure(new ElasticsearchStatusException("Failed to test query", RestStatus.SERVICE_UNAVAILABLE, e))));
9497
}
9598

9699
public void deduceMappings(Client client, SourceConfig sourceConfig, final ActionListener<Map<String, String>> listener) {

‎x-pack/plugin/data-frame/src/test/java/org/elasticsearch/xpack/dataframe/transforms/pivot/PivotTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
package org.elasticsearch.xpack.dataframe.transforms.pivot;
88

99
import org.apache.lucene.search.TotalHits;
10+
import org.elasticsearch.ElasticsearchException;
1011
import org.elasticsearch.action.ActionType;
1112
import org.elasticsearch.action.ActionListener;
1213
import org.elasticsearch.action.ActionRequest;
@@ -144,7 +145,7 @@ public void testValidateAllUnsupportedAggregations() throws Exception {
144145
AggregationConfig aggregationConfig = getAggregationConfig(agg);
145146

146147
Pivot pivot = new Pivot(getValidPivotConfig(aggregationConfig));
147-
RuntimeException ex = expectThrows(RuntimeException.class, pivot::validateConfig);
148+
ElasticsearchException ex = expectThrows(ElasticsearchException.class, pivot::validateConfig);
148149
assertThat("expected aggregations to be unsupported, but they were", ex, is(notNullValue()));
149150
}
150151
}

‎x-pack/plugin/src/test/resources/rest-api-spec/test/data_frame/preview_transforms.yml

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,3 +252,35 @@ setup:
252252
}
253253
}
254254
}
255+
---
256+
"Test preview with unsupported agg":
257+
- do:
258+
catch: bad_request
259+
data_frame.preview_data_frame_transform:
260+
body: >
261+
{
262+
"source": { "index": "airline-data" },
263+
"dest": { "pipeline": "missing-pipeline" },
264+
"pivot": {
265+
"group_by": {
266+
"time": {"date_histogram": {"fixed_interval": "1h", "field": "time"}}},
267+
"aggs": {
268+
"vals": {"terms": {"field":"airline"}}
269+
}
270+
}
271+
}
272+
- do:
273+
catch: /Unsupported aggregation type \[terms\]/
274+
data_frame.preview_data_frame_transform:
275+
body: >
276+
{
277+
"source": { "index": "airline-data" },
278+
"dest": { "pipeline": "missing-pipeline" },
279+
"pivot": {
280+
"group_by": {
281+
"time": {"date_histogram": {"fixed_interval": "1h", "field": "time"}}},
282+
"aggs": {
283+
"vals": {"terms": {"field":"airline"}}
284+
}
285+
}
286+
}

0 commit comments

Comments
 (0)