Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove delimited_payload_filter #27705

Merged
merged 1 commit into from
Apr 5, 2018
Merged

Remove delimited_payload_filter #27705

merged 1 commit into from
Apr 5, 2018

Conversation

cbuescher
Copy link
Member

From 7.0 on, using delimited_payload_filter should throw an error (the use was deprecated in 6.2 in favour of delimited_payload in #26625.

Relates to #27704

@cbuescher cbuescher added :Search Relevance/Analysis How text is split into tokens v7.0.0 labels Dec 7, 2017
@cbuescher cbuescher force-pushed the error-on-delimited_payload_filter branch from ec17144 to 7f64c6e Compare December 7, 2017 16:51
@cbuescher cbuescher changed the title Error when using delimited_payload_filter from 7.0.0 on Remove delimited_payload_filter Dec 7, 2017
@cbuescher cbuescher force-pushed the error-on-delimited_payload_filter branch from 7f64c6e to 28d3e05 Compare December 8, 2017 10:57
@mayya-sharipova
Copy link
Contributor

@cbuescher LGTM, +1 after rebasing.

Also, a general question: after we rename delimited_payload_filter, I assume in the version 8 we can completely get rid of LegacyDelimitedPayloadTokenFilterFactory.java class?

So, does the progression follows from deprecation -> error -> removing not used code?

@cbuescher cbuescher force-pushed the error-on-delimited_payload_filter branch from 28d3e05 to 80d9427 Compare January 31, 2018 09:43
@cbuescher
Copy link
Member Author

@mayya-sharipova thanks for the review, I rebase to get a new CI run. I will hold back merging this for a bit because I also want to check what needs to be done to add this to the migration assistant as part of #27704

cbuescher
cbuescher commented Feb 5, 2018
View reviewed changes
docs/reference/migration/migrate_7_0/analysis.asciidoc Outdated

The `delimited_payload_filter` was deprecated and renamed to `delimited_payload` in 6.2.
Using it indices created before 7.0 will issue deprecation warnings. Using the old
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not to self: "in" missing after "Using it"

cbuescher
cbuescher commented Feb 5, 2018
View reviewed changes
docs/reference/migration/migrate_7_0/analysis.asciidoc Outdated
The `delimited_payload_filter` was deprecated and renamed to `delimited_payload` in 6.2.
Using it indices created before 7.0 will issue deprecation warnings. Using the old
name in new indices created in will throw an error. Use the new name `delimited_payload`
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, somethings missing after "created in"

@romseygeek
Copy link
Contributor

Can this be merged @cbuescher ?

cc @elastic/es-search-aggs

@cbuescher cbuescher force-pushed the error-on-delimited_payload_filter branch from 80d9427 to 783749a Compare March 14, 2018 14:17
@cbuescher
Copy link
Member Author

@romseygeek almost ready to go, but I still want to check if anything needs to be done with regard to #26625 (comment) where adding some checks to the migration assistant is mentioned. Don't know what to do about it yet.

@cbuescher cbuescher self-assigned this Mar 14, 2018
@cbuescher
Copy link
Member Author

Merging this now that adding this change to the migration assistant is tracked elsewhere.

@cbuescher cbuescher merged commit 231fd4e into elastic:master Apr 5, 2018
@cbuescher cbuescher mentioned this pull request Apr 5, 2018
Closed
@gwbrown gwbrown mentioned this pull request Nov 29, 2018
Closed
26 tasks
russcam added a commit to elastic/elasticsearch-net that referenced this pull request Jun 20, 2019
@russcam
Support deprecated delimited_payload_filter name in deserialization
d1aa5b4 
Relates: elastic/elasticsearch#27705

This commit adds support for the deprecated delimited_payload_filter
name when deserializing token filters in a 6.x index within a 7.x cluster.

When serializing, the new name delimited_payload will be used.
@russcam russcam mentioned this pull request Jun 20, 2019
Merged
Mpdreamz pushed a commit to elastic/elasticsearch-net that referenced this pull request Jun 20, 2019
@russcam @Mpdreamz
Support deprecated delimited_payload_filter name in deserialization (#…
21b958d 
…3835)

Relates: elastic/elasticsearch#27705

This commit adds support for the deprecated delimited_payload_filter
name when deserializing token filters in a 6.x index within a 7.x cluster.

When serializing, the new name delimited_payload will be used.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mayya-sharipova mayya-sharipova mayya-sharipova approved these changes

Assignees

@cbuescher cbuescher

Labels
>breaking :Search Relevance/Analysis How text is split into tokens v7.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@cbuescher @mayya-sharipova @romseygeek @colings86 @jpountz