Update Scripting API to Store Context for Scripts #20621
Conversation
Note any context currently passed in will not be used. This is simply a precursor to requiring context at compilation time. This also changes stored scripts to have a single namespace of id rather than lang/id. The lang will now be retrieved from the stored script rather than need to be passed in once the script is stored.
jdconrad
added
WIP
:Core/Infra/Scripting
|
I dropped |
| boolean exists = in.readBoolean(); | ||
|
|
||
| if (exists) { | ||
| source = StoredScriptSource.staticReadFrom(in); |
There was a problem hiding this comment.
I think it'd be more normal to do in.readOptionalWriteable and make sure StoredScriptSource is Writeable.
| public int hashCode() { | ||
| return Objects.hash(lang, params, script, type, contentType); | ||
| public interface ScriptBinding { | ||
| Object compile(ScriptEngineService engine, String id, String code, Map<String, String> options); |
There was a problem hiding this comment.
Should this be ScriptBinding<T> where T is the interface that we expect the script to implement? Then compile can return T?
| ParseField SCRIPT = new ParseField("script"); | ||
| ParseField LANG = new ParseField("lang"); | ||
| ParseField PARAMS = new ParseField("params"); | ||
| public static final class SearchScriptBinding implements ScriptBinding { |
There was a problem hiding this comment.
I feel like this should be somewhere in the search infrastructure rather.
| } | ||
|
|
||
| public static final String DEFAULT_SCRIPT_LANG = "painless"; | ||
| public static final String DEFAULT_SCRIPT_NAME = "<inline>"; |
There was a problem hiding this comment.
Maybe rename to INLINE_SCRIPT_NAME?
|
|
||
| public static final String CONTENT_TYPE_OPTION = "content_type"; | ||
|
|
||
| private Script() {} |
There was a problem hiding this comment.
Can you add a comment about why you need the empty private ctor?
|
I'm going to separate out this change into smaller pieces. I'm going to close this for now. |
Change stored scripts/file scripts to take in a context, and ensure that context is available at compile-time as a precursor to this feature (#20426).
This is currently a WIP since it's a large change, and there's some way to go. I wanted to get early feedback on this portion.
The steps for this change are the following: