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

Created a parameter parser to standardise script options #7977

Merged
merged 1 commit into from Oct 8, 2014
Merged

Created a parameter parser to standardise script options #7977

merged 1 commit into from Oct 8, 2014

Conversation

colings86
Copy link
Contributor

No description provided.

@colings86 colings86 changed the title Scripting: Created a parameter parse to standardised script options Scripting: Created a parameter parser to standardised script options Oct 3, 2014
@colings86 colings86 self-assigned this Oct 3, 2014
@s1monw
Copy link
Contributor

s1monw commented Oct 6, 2014

LGTM

@s1monw s1monw removed the review label Oct 6, 2014
@colings86
Copy link
Contributor Author

@s1monw thanks for the review, unfortunately I had forgotten to make the scripted metric aggregation use the ScriptParameterParser so have added a new commit to the PR

@s1monw
Copy link
Contributor

s1monw commented Oct 7, 2014

LGTM still :)

@colings86 colings86 merged commit c047fbd into elastic:master Oct 8, 2014
@colings86 colings86 deleted the feature/consistentScriptParsing branch October 8, 2014 08:24
@jpountz jpountz removed the review label Oct 21, 2014
@clintongormley clintongormley changed the title Scripting: Created a parameter parser to standardised script options Scripting: Created a parameter parser to standardise script options Nov 4, 2014
@clintongormley clintongormley added the :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache label Mar 19, 2015
@javanna
Copy link
Member

javanna commented Apr 4, 2015

@colings86 this change was backported to 1.x in a bw comp manner but breaks backwards compatibility on master I think, as only the new parameters get read on master now, is that right?

If so this PR might deserve the breaking label (although confusing cause it's breaking only on master branch).

@colings86
Copy link
Contributor Author

This is only a breaking change in 2.0

@sn00011
Copy link

sn00011 commented Apr 16, 2015

Is there any get around, for 1.4.4?

When i use the script fields "(init_script, map_script, combine_script, reduce_script)" it throws me the following error:

 ScriptException[dynamic scripting for [groovy] disabled]; ]

And when i use the *_script_file fields, it throws me:

Unable to find on disk script 

So now i can't use the scripted metric aggregation on 1.4.4

@colings86
Copy link
Contributor Author

@kurtzhong I just tried the following gist on Elasticsearch 1.4.4 with the default config (apart form enabling CORS and setting the cluster name) and it worked. Could you try this on your cluster and let me know if it works?

The location of the scripts is very important, they must be in a folder called scripts in your config folder (where your elasticsearch.yml file is) and they must have the correct extension for the language you are using (i.e. .groovy if you are using the default language, groovy).

@jveldboom
Copy link

We had to explicitly set the lang to groovy for it to work.

{
    "script_score": {
        "params": {
            "mult": 1.2
        },
        "script": "sales-ytd",
        "lang":"groovy"
    }
}

@clintongormley clintongormley changed the title Scripting: Created a parameter parser to standardise script options Created a parameter parser to standardise script options Jun 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants