Configurable MIME type for mustache template encoding on set processor #65314
Conversation
danhermann
added
>enhancement
:Data Management/Ingest Node
elasticmachine
added
the
Team:Data Management
|
Pinging @elastic/es-core-features (Team:Core/Features) |
|
@elasticmachine update branch |
There was a problem hiding this comment.
LGTM, thanks @danhermann !
We should add documentation for this new property either in this PR or in the follow up
|
Thank you, @probakowski! I will open another PR shortly with documentation for this option. |
| @@ -110,10 +111,11 @@ public SetProcessor create(Map<String, Processor.Factory> registry, String proce | |||
| String description, Map<String, Object> config) throws Exception { | |||
| String field = ConfigurationUtils.readStringProperty(TYPE, processorTag, config, "field"); | |||
| String copyFrom = ConfigurationUtils.readOptionalStringProperty(TYPE, processorTag, config, "copy_from"); | |||
| String mimeType = ConfigurationUtils.readMimeTypeProperty(TYPE, processorTag, config, "mime_type", "application/json"); | |||
There was a problem hiding this comment.
are we sure we want mime_type as a name here? I thought mime was replace with media_type
but wouldn't content_type be more consistent with what we have in mustache templating engine?
There was a problem hiding this comment.
or we could consider to rename the content_type to media_type (in mustache template engine)?
There was a problem hiding this comment.
I'm happy to rename to whatever is most appropriate. I went with mime_type because that is what was used in CustomMustacheFactory but I would agree that media_type or possibly content_type would be better.
There was a problem hiding this comment.
+1 to follow up with a change to media_type to be more correct.
| @@ -49,6 +49,7 @@ | |||
|
|
|||
| public static final String TAG_KEY = "tag"; | |||
| public static final String DESCRIPTION_KEY = "description"; | |||
| public static final String[] VALID_MIME_TYPES = {"application/json", "text/plain", "application/x-www-form-urlencoded"}; | |||
There was a problem hiding this comment.
do we want the se hardcoded? What if someone specifies application/json; charset=utf-8 ?
Would it be of any help if this bit was unified with what CustomMustacheFactory defines? I made an attempt here https://github.com/elastic/elasticsearch/pull/66857/files#diff-207cacc1e4a5b24109a693c0b63dd762156452bb5984b5ce04499b28e6c191e8R28
There was a problem hiding this comment.
I agree that unifying this set of options would be good. I can do that after you merge that PR. I don't believe that we want to allow the specification of a particular character set for the use case of the ingest processor, though.
|
@elastic/es-ui, is there work required on the UI side to update Kibana auto-complete with the new |
|
@danhermann Yes, we'll update Console to autocomplete with this new option when we regenerate all of the autocomplete definitions for 7.12. |
The set processor permits the value that it sets to be specified with a Mustache template snippet. Our Mustache script engine allows three different MIME encodings to be used when writing the value that the snippet evaluates to but the Set processor was hard-coded to always use the
application/jsonMIME type. This adds an optionalmime_typeproperty to the Set processor that may be set to any of the three MIME types (application/json,text/plain,application/x-www-form-urlencoded) that the Mustache script engine supports. The default type remainsapplication/json.Closes #65115