Update UpdateForV9 in AttachmentProcessor
#118186
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Documentation preview: |
PeteGillinElastic
force-pushed
the
v9-attachment-processor-remove-binary-cleanup
branch
from
ffe8647 to
c13b7c7
Compare
PeteGillinElastic
added
>non-issue
:Data Management/Ingest Node
We are not going to make this change in V9. We may do it in V10. This change just bumps the annotation to remind us to revisit. Since we are living with this for a while, it seems worth improving the documentation. This now encourages explicitly setting the option one way or the other, since you get a warning if you omit it. It also changes the existing examples to use true rather than false, as that's our recommendation. And it adds a new section with an example where it's true, and moves the content previously in a note into that section.
PeteGillinElastic
force-pushed
the
v9-attachment-processor-remove-binary-cleanup
branch
from
c13b7c7 to
ebc64bc
Compare
|
Pinging @elastic/es-data-management (Team:Data Management) |
elasticsearchmachine
added
the
Team:Data Management
| | `indexed_chars_field` | no | `null` | Field name from which you can overwrite the number of chars being used for extraction. See `indexed_chars`. | ||
| | `properties` | no | all properties | Array of properties to select to be stored. Can be `content`, `title`, `name`, `author`, `keywords`, `date`, `content_type`, `content_length`, `language` | ||
| | `ignore_missing` | no | `false` | If `true` and `field` does not exist, the processor quietly exits without modifying the document | ||
| | `remove_binary` | encouraged | `false` | If `true`, the binary `field` will be removed from the document. This option is not required, but setting it explicitly is encouraged, and omitting it will result in a warning. |
There was a problem hiding this comment.
I wasn't sure of the best way to convey this state of affairs... Alternative suggestions welcome!
…ssor-remove-binary-cleanup
| @UpdateForV9(owner = UpdateForV9.Owner.DATA_MANAGEMENT) | ||
| // update the [remove_binary] default to be 'true' assuming enough time has passed. Deprecated in September 2022. | ||
| @UpdateForV10(owner = UpdateForV10.Owner.DATA_MANAGEMENT) | ||
| // Revisit whether we want to update the [remove_binary] default to be 'true' - would need to find a way to do this safely |
There was a problem hiding this comment.
N.B. I'm not currently changing the deprecation message (which says that the property "will be set to 'true' in a future release", when actually we're not committed to that) because we don't know for sure what we're going to do in V10 and it seems better to (a) stay with the status quo, and (b) err on the side of making an overly-strong warning. I'm open to suggestions for rewording, though.
masseyke
approved these changes
Dec 9, 2024
|
Thanks @masseyke! |
PeteGillinElastic
deleted the
v9-attachment-processor-remove-binary-cleanup
branch
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
PeteGillinElastic
added a commit
to PeteGillinElastic/elasticsearch
that referenced
this pull request
Dec 9, 2024
We are not going to make this change in V9. We may do it in V10. This change just bumps the annotation to remind us to revisit. Since we are living with this for a while, it seems worth improving the documentation. This now encourages explicitly setting the option one way or the other, since you get a warning if you omit it. It also changes the existing examples to use true rather than false, as that's our recommendation. And it adds a new section with an example where it's true, and moves the content previously in a note into that section. (cherry picked from commit bc25a73) # Conflicts: # modules/ingest-attachment/src/main/java/org/elasticsearch/ingest/attachment/AttachmentProcessor.java
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
PeteGillinElastic
added a commit
that referenced
this pull request
Dec 9, 2024
Improve the docs around `remove_binary` in `attachment` Since we are living with this for a while, it seems worth improving the documentation. This now encourages explicitly setting the option one way or the other, since you get a warning if you omit it. It also changes the existing examples to use true rather than false, as that's our recommendation. And it adds a new section with an example where it's true, and moves the content previously in a note into that section. (cherry picked from commit bc25a73) # Conflicts: # modules/ingest-attachment/src/main/java/org/elasticsearch/ingest/attachment/AttachmentProcessor.java
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We are not going to make this change in V9. We may do it in V10. This change just bumps the annotation to remind us to revisit.
Since we are living with this for a while, it seems worth improving the documentation. This now encourages explicitly setting the option one way or the other, since you get a warning if you omit it. It also changes the existing examples to use true rather than false, as that's our recommendation. And it adds a new section with an example where it's true, and moves the content previously in a note into that section.