-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
Hardcode painless as the default scripting lang and add legacy script default for stored scripts #20310
Conversation
} | ||
|
||
public static String getLegacyDefaultLang(Settings settings) { | ||
return settings.get("script.legacy.default_lang", ScriptSettings.LEGACY_DEFAULT_LANG); |
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 key is different from the one specified above in https://github.com/elastic/elasticsearch/pull/20310/files#diff-9a1a4781a78e7e7a89657e8717f88997R61
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.
oops, will fix
I really think we need a test for this. maybe we can build a bwc index with a script in the percolator? Maybe we can even fake it in a unittest and build a lucene index ourself, start an indexservice etc. to test this. Otherwise this looks awesome |
I think we can remove the WIP label here too |
@s1monw I've updated the PR and replaced the WIP label with review label. |
LGTM great solution, good tests, well done! thanks mate! |
13e42a7
to
2b2bfbc
Compare
…oded default script language. ** The default script language is now maintained in `Script` class. * Added `script.legacy.default_lang` setting that controls the default language for scripts that are stored inside documents (for example percolator queries). This defaults to groovy. ** Added `QueryParseContext#getDefaultScriptLanguage()` that manages the default scripting language. Returns always `painless`, unless loading query/search request in legacy mode then the returns what is configured in `script.legacy.default_lang` setting. ** In the aggregation parsing code added `ParserContext` that also holds the default scripting language like `QueryParseContext`. Most parser don't have access to `QueryParseContext`. This is for scripts in aggregations. * The `lang` script field is always serialized (toXContent). Closes elastic#20122
2b2bfbc
to
245882c
Compare
script.default_lang
setting and madepainless
the hardcoded default script language.Script
class.script.legacy.default_lang
setting that controls the default language for scripts that are stored inside documents (for example percolator queries). This defaults to groovy.QueryParseContext#getDefaultScriptLanguage()
that manages the default scripting language. Returns alwayspainless
, unless loading query/search request in legacy mode then the returns what is configured inscript.legacy.default_lang
setting.ParserContext
that also holds the default scripting language likeQueryParseContext
. Most parser don't have access toQueryParseContext
. This is for scripts in aggregations.lang
script field is always serialized (toXContent).PR for #20122
Need to add tests and docs, but before adding those I'd like to get some feedback.