Allow scripted metric agg to access _score
#24295
Conversation
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
|
jenkins test this |
colings86
left a comment
colings86
left a comment
There was a problem hiding this comment.
@winstonewert Thanks for raising this PR. I left a couple of comments.
There was a problem hiding this comment.
Could you have the score scripts as separate scripts rather than repurposing the existing ones? I think its important that we continue testing scripts that don't use score as well as scripts that do.
There was a problem hiding this comment.
I think we should test cases with score separately from those without scores and retain both types of test.
|
I've pushed a new commit fixing the issues mentioned. Thanks for your time in reviewing it. |
|
jenkins retest this |
clintongormley
changed the title
_score
|
I see that the build failed. But it looks like there was an unrelated change in the test? I'm guessing it has something to do with changes having been made since I forked? But I'm not sure how to update my version without breaking the review process here. |
There was a problem hiding this comment.
@winstonewert Thanks for updating the tests I think this looks good now. Could you rebase your branch with the latest master branch (git rebase master) and we'll give the tests another go at running before we merge?
Corrects the ScriptedMetricAggregator so that the script can have access to scores during the map stage.
As requested, I've restored the non-score dependant tests, and added the score dependent metric as a seperate test.
|
jenkins retest this |
|
I haven't rebased much in the past, and I had to try several times to get it right. Let me know if I still got it wrong. |
|
@winstonewert that looks right now, thanks. I think I was a bit fast kicking off that build so will do it again |
|
jenkins retest this |
* Fixes #24259 Corrects the ScriptedMetricAggregator so that the script can have access to scores during the map stage. * Restored original tests. Added seperate test. As requested, I've restored the non-score dependant tests, and added the score dependent metric as a seperate test.
* Fixes #24259 Corrects the ScriptedMetricAggregator so that the script can have access to scores during the map stage. * Restored original tests. Added seperate test. As requested, I've restored the non-score dependant tests, and added the score dependent metric as a seperate test.
|
@winstonewert thanks for raising this PR. I have merged and backported this and it will be available from the 5.4.1 release. Thanks for your contribution |
* master: (27 commits) Check index sorting with no replica since we cannot ensure that the replica index is ready when forceMerge is called. Closes elastic#24416 Docs: correct indentation on callout Build that java api docs from a test (elastic#24354) Move RemoteClusterService into TransportService (elastic#24424) Fix license header in WildflyIT.java Try not to lose stacktraces (elastic#24426) [DOCS] Update XPack Reference URL for 5.4 (elastic#24425) Painless: Add tests to check for existence and correct detection of the special Java 9 optimizations: Indified String concat and MethodHandles#ArrayLengthHelper() (elastic#24405) Extract a common base class to allow services to listen to remote cluster config updates (elastic#24367) Adds check to snapshot repository incompatible-snapshots blob to delete a pre-existing one before attempting to overwrite it. Added docs for batched_reduce_size Fixes checkstyle errors Allow scripted metric agg to access `_score` (elastic#24295) [Test] Add unit tests for HDR/TDigest PercentilesAggregators (elastic#24245) Fix FieldCaps documentation Upgrade to JUnit 4.12 (elastic#23877) Set available processors for Netty Painless: Fix method references to ctor with the new LambdaBootstrap and cleanup code (elastic#24406) Doc test: use propery regex for file size [DOCS] Tweak doc test to sync_flush ...
4 participants
Corrects the ScriptedMetricAggregator so that the script can have
access to scores during the map stage.