-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Add deprecation check for processors #45925
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.
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
Add deprecation check for processors #45925
Conversation
The processors setting is deprecated. This commit adds a deprecation check for the use of the processors setting.
|
Pinging @elastic/es-core-infra |
| "setting [%s] is deprecated in favor of setting [%s]", | ||
| EsExecutors.PROCESSORS_SETTING.getKey(), | ||
| EsExecutors.NODE_PROCESSORS_SETTING.getKey()); | ||
| final String url = "https://www.elastic.co/guide/en/elasticsearch/reference/master/breaking-changes-8.0.html#remove-processors"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This link refers to master, which may be different when someone sees this setting (for example once 7.x is in maintenance mode)? I think we need a better permalink if we are to include this, but haven't we avoided links in exception messages in the past for this exact reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me make a docs push to add a deprecation note to the 7.4 docs which can be used as a permalink.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've just realized we have the same issue for the existing DeprecationIssue instances, which we should probably (separately from this PR) fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gwbrown Can you address that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rjernst I pushed a permalink that is currently broken, but will be fixed by the time the 7.4 release is in published.
rjernst
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The processors setting is deprecated. This commit adds a deprecation check for the use of the processors setting.
Relates #45855
Relates #45905