Remove deprecated code from stored scripts #78643
Conversation
jdconrad
added
:Core/Infra/Scripting
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
|
@jrodewig Would you please review the migration docs I've added here? Thanks in advance! |
| // since there is a guarantee no more empty scripts will exist | ||
| if (source.getSource().isEmpty()) { | ||
| if (Script.DEFAULT_TEMPLATE_LANG.equals(source.getLang())) { | ||
| deprecationLogger.critical(DeprecationCategory.TEMPLATES, "empty_templates", |
There was a problem hiding this comment.
I think these should be warning logs, not deprecations. The deprecation already happened, this is a warning that we are dropping something meaningless.
| @@ -22,4 +22,29 @@ scripts. Any use of `getDayOfWeek` expecting a return value of `int` will result | |||
| in a compilation error or runtime error and may not allow the upgraded node to | |||
| start. | |||
| ==== | |||
|
|
|||
| .Empty scripts/templates are no longer allowed as stored scripts. | |||
There was a problem hiding this comment.
| .Empty scripts/templates are no longer allowed as stored scripts. | |
| .Stored scripts no longer support empty scripts or search templates. |
| Empty scripts/templates cannot be specified as the source of a | ||
| PutStoredScript action. |
There was a problem hiding this comment.
Is there a reason we reference the PutStoredScript action rather than the create or update stored script API here?
It doesn't look like it in the code, but the wording made me wonder if this only impacted the Java client.
There was a problem hiding this comment.
No! Just a mistake on my part.
| Before upgrading, remove stored scripts/templates that are empty. Otherwise, | ||
| these scripts will be dropped, and may not align with the scripts on older | ||
| nodes. |
There was a problem hiding this comment.
I added a link to the delete stored script API. I assume the user will use that API to remove the scripts.
| Before upgrading, remove stored scripts/templates that are empty. Otherwise, | |
| these scripts will be dropped, and may not align with the scripts on older | |
| nodes. | |
| Before upgrading, use the {ref}/delete-stored-script-api.html[delete stored | |
| script API] to delete any empty stored scripts or search templates. | |
| In 8.0, {es} will drop any empty stored scripts or empty search templates from | |
| the cluster state. Requests to create a stored script or search template with | |
| an empty `source` will return an error. |
and may not align with the scripts on older nodes.
My suggestion doesn't include this because I wasn't sure what it meant. Does it just mean that behavior will be different after the upgrade?
There was a problem hiding this comment.
Yes. Poor wording on my part. I'm adopting your suggestion as the entirety of the text here.
| nodes. | ||
| ==== | ||
|
|
||
| .The `code` field can no longer be specified as part of a stored script/template. |
There was a problem hiding this comment.
| .The `code` field can no longer be specified as part of a stored script/template. | |
| .The create or update stored script API's `code` parameter has been removed. |
| The `code` field is replaced by the `source` field as the only way to specify | ||
| the source of a script. |
There was a problem hiding this comment.
| The `code` field is replaced by the `source` field as the only way to specify | |
| the source of a script. | |
| The {ref}/create-stored-script-api.html[create or update stored script API]'s | |
| `code` parameter has been removed. Use the `source` parameter instead. |
| Before upgrading, any calls to PutStoredScript should use `source` instead | ||
| of `code`. |
There was a problem hiding this comment.
| Before upgrading, any calls to PutStoredScript should use `source` instead | |
| of `code`. | |
| Discontinue use of the `code` parameter. Requests that include the parameter | |
| will return an error. |
|
@jrodewig Thanks for the detailed review! I have merged your feedback and updated the details for section where script source cannot be empty. Ready for the next round. |
This change removes several pieces of deprecated code from stored scripts.
Stored scripts/templates are no longer allowed to be an empty and will throw an exception when used with PutStoredScript.
ScriptMetadata will now drop any existing stored scripts that are empty with a deprecation warning in the case they have not been previously removed.
The
codefield is now only allowed assourceas part of a PutStoredScript JSON blob.