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.

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

Requests do not always propagate headers/context when retrieving indexed scripts #12891

Closed
jaymode opened this issue Aug 14, 2015 · 1 comment
Closed

Requests do not always propagate headers/context when retrieving indexed scripts #12891

jaymode opened this issue Aug 14, 2015 · 1 comment
Assignees
@colings86
Labels
:Analytics/Aggregations Aggregations blocker >bug :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache :Search/Search Search-related issues that do not fall into other categories v2.0.0-beta2

Comments

@jaymode
Copy link
Member

jaymode commented Aug 14, 2015

In #11060 (issue #10979), the request headers and context of the originating request were copied to all sub-requests. In doing some testing on master, I noticed that in the ScriptService#getScriptFromIndex the headers and context were not being copied due to the lack of a SearchContext. To see what calls were affected, I added an assert to check if the context was null. When doing so the following tests trip the assertion:

- org.elasticsearch.search.aggregations.pipeline.BucketScriptIT.indexedScript
- org.elasticsearch.search.aggregations.metrics.ScriptedMetricIT.testInitMapCombineReduce_withParams_Indexed
- org.elasticsearch.search.aggregations.pipeline.BucketSelectorIT.indexedScript
- org.elasticsearch.validate.RenderSearchTemplateIT.indexedTemplate

There were also two rest test failures for the Render Search Template API. For the render search template api, we need a way to pass in the headers/context in a way that isn't a search context because it is not a search request.

For the other failures, it appears that the subsequent requests happen after the SearchContext has been cleared/released.
@jaymode jaymode added >bug :Analytics/Aggregations Aggregations :Search/Search Search-related issues that do not fall into other categories :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache :Internal v2.0.0 labels Aug 14, 2015
@clintongormley
Copy link

@colings86 please could you take this.

@colings86 colings86 mentioned this issue Aug 19, 2015
Merged
colings86 added a commit that referenced this issue Sep 2, 2015
@colings86
Scripting: Propagate Headers and Context through to ScriptService
43b33fb 
At the moment if an index script is used in a request, the spawned request to get the indexed script from the `.scripts` index does not get the headers and context copied to it from the original request. This change makes the calls to the `ScriptService` pass in a `HasContextAndHeaders` object that can provide the headers and context. For the `search()` method the context and headers are retrieved from `SearchContext.current()`.

Closes #12891
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@colings86 colings86

Labels
:Analytics/Aggregations Aggregations blocker >bug :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache :Search/Search Search-related issues that do not fall into other categories v2.0.0-beta2
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@clintongormley @colings86 @jaymode