[Transform] Preview index request #137455
Conversation
Preview currently shows the source document as the index request. There are some internal components that want both the source document and the id of the generated entities, so we've added a query param that controls the output format. `as_index_request` defaults to false to show the existing structure, and true will show the response that the transform would use to index the generated entities.
prwhelan
added
>enhancement
:ml/Transform
|
Hi @prwhelan, I've created a changelog YAML for you. |
|
Pinging @elastic/ml-core (Team:ML) |
benwtrent
left a comment
benwtrent
left a comment
There was a problem hiding this comment.
Makes sense to me. However, we should throw if not supported by the transport layer.
| if (out.getTransportVersion().supports(PREVIEW_AS_INDEX_REQUEST)) { | ||
| out.writeBoolean(previewAsIndexRequest); | ||
| } |
There was a problem hiding this comment.
Let's throw if somebody specifically requested true, but the transport version doesn't support it. This way they don't think they ran into a bug, but instead their cluster just isn't fully upgraded yet.
There was a problem hiding this comment.
Is there an example of this kind of thing elsewhere? This would effectively break the API until they're on the same version - would it be easier/better to use NodeFeature to instead disable the feature until every node is on the latest version?
There was a problem hiding this comment.
This would effectively break the API until they're on the same version - would it be easier/better to use NodeFeature to instead disable the feature until every node is on the latest version?
No it wouldn't. Folks can still use false which is the default value. Why would we want to silently fail the request if somebody specifically asked for a new feature that we cannot provide?
There was a problem hiding this comment.
would it be easier/better to use NodeFeature
This I do not know, but that has additional guarantees and costs that we don't really need.
I think transport version is perfectly fine. I am saying that the default behavior of the API should still be usable, but if somebody sends a parameter that is not supported by nodes that will receive the message, we should not silently ignore it and instead tell the user "Hey, this feature you want to use isn't available yet"
This has been done before by other APIs in the past, I don't have any examples handy.
| "_preview with " | ||
| + TransformField.PREVIEW_AS_INDEX_REQUEST.getPreferredName() | ||
| + " set to true only works if all the nodes support it.", | ||
| RestStatus.FORBIDDEN |
There was a problem hiding this comment.
Sure - in the past I've used 403 or 422 for this, since the request was understood and syntactically correct, but the server is not going to process it. 400 has more of a "I don't know what you just said to me" vibe.
BASE=cb11a7164033f79a0c401a85ca8db22105631888 HEAD=078cff9adadabdb362d925d026037fe3ae84fed7 Branch=main
BASE=18dc946cc969eed4385e71085a6fa3f4198fa534 HEAD=b16caf7c0183196d8b034e090ca0561ade060a4e Branch=main
Preview currently shows the source document as the index request. There are some internal components that want both the source document and the id of the generated entities, so we've added a query param that controls the output format. `as_index_request` defaults to false to show the existing structure, and true will show the response that the transform would use to index the generated entities.
3 participants
Preview currently shows the source document as the index request. There are some internal components that want both the source document and the id of the generated entities, so we've added a query param that controls the output format.
as_index_requestdefaults to false to show the existing structure, and true will show the response that the transform would use to index the generated entities.