[ML]add InferenceAction request query validation #110147
Conversation
|
💚 CLA has been signed |
There was a problem hiding this comment.
Hi @Huaixinww
Thank you for the PR! It would be great, if you could sign the contributor agreement to move forward on the review. For changes in the inference api you can also add the two label :ml and Team:ML to request a review from the ML team.
| if (taskType.equals(TaskType.RERANK)) { | ||
| if (query == null) { | ||
| var e = new ActionRequestValidationException(); | ||
| e.addValidationError("missing query"); |
There was a problem hiding this comment.
| e.addValidationError("missing query"); | |
| e.addValidationError(format("Field [query] cannot be null for task type [%s]", TaskType.RERANK)); |
| } | ||
| if (query.isEmpty()) { | ||
| var e = new ActionRequestValidationException(); | ||
| e.addValidationError("query is empty"); |
There was a problem hiding this comment.
I think we can also use something like e.addValidationError(format("Field [query] cannot be empty for task type [%s]", TaskType.RERANK)); similar to my comment above
| @@ -71,6 +72,84 @@ public void testParsing() throws IOException { | |||
| } | |||
| } | |||
|
|
|||
| public void testValidation() throws IOException { | |||
There was a problem hiding this comment.
I think this test could be split into several smaller tests testValidation_TextEmbedding_Null, testValidation_TextEmbedding_Empty, testValidation_Rerank_Null etc.
|
Pinging @elastic/ml-core (Team:ML) |
|
Hi @timgrein |
|
@elasticmachine test this please |
When testing Cohere's rerank function, we discovered that if a query is not provided, cohereRerankRequest will throw a NPE.
The validate() method of InferenceAction checks the input but does not validate the query, causing cohereRerankRequest to throw a NPE because it requires the query to be non-null.