Sync the API definition for put_script with the docs by honzakral · Pull Request #25736 · elastic/elasticsearch

honzakral · 2017-07-15T14:59:30Z

According to the docs (0) the lang portion of the URL is not required, also it only 1 parameter is supplied, it should be the id, not lang.

0 - https://www.elastic.co/guide/en/elasticsearch/reference/current/modules-scripting-using.html#_request_examples

jdconrad · 2017-07-17T14:52:00Z

@martijnvg Please correct me if I'm wrong here, but I believe this cannot be changed since the url paths much match based on the parameter number. See the explanation in the comment here (https://github.com/elastic/elasticsearch/blob/master/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestPutStoredScriptAction.java) on line 40. I don't really know how to resolve the fact that the documentation is generated from this file as well.

martijnvg · 2017-07-20T07:31:09Z

I think for 5.x we should keep the lang parameter as it is still handled on the ES side?
The #25779 pr actually removes lang parameter from the code and the spec.

jdconrad · 2017-07-20T23:50:31Z

@honzakral @martijnvg The problem is there are two paths in 5.x which is confusing /{lang} and /{lang}/{id} where in the first path lang actually means id. Nothing can easily be done about this, however, and it is gone in 6.0, so I think this should probably be closed.

honzakral · 2017-07-21T00:51:54Z

At that point we arrive at the theoretical question - what does the api spec and the accompanying yaml tests model? The internal java implementation, or the API?

The api is quite clear - if only one argument is given, it is interpreted as id not as lang. The fact that you call it lang internally is of no consequence to the actual API and it's documentation, usage, and meaning.

rjernst · 2017-07-21T16:12:32Z

The fact that you call it lang internally is of no consequence

It is of immense consequence to the actual implementation of how the rest test runner works. As @jdconrad explained, the spec handling by the test runner expects variations like this to be appending new spec variables, not modifying existing ones.

I don't think this is worthwhile to try to fix in 5.x. I think we should just live with it until 6.0 is released.

clintongormley · 2017-07-24T13:07:03Z

btw, putting a script with the lang in the URL does not work on 5.x today.

This works:

PUT _scripts/calculate-score
{
  "script": {
    "lang": "painless",
    "source": "Math.log(_score * 2) + params.my_modifier"
  }
}

This doesn't work:

PUT _scripts/painless/calculate-score
{
  "script": {
    "lang": "painless",
    "source": "Math.log(_score * 2) + params.my_modifier"
  }
}

Instead it returns:

{
  "error": {
    "root_cause": [
      {
        "type": "script_exception",
        "reason": "compile error",
        "script_stack": [
          "{\"source\":\"Math.log(_scor ...",
          "^---- HERE"
        ],
        "script": "{\"source\":\"Math.log(_score * 2) + params.my_modifier\"}",
        "lang": "painless"
      }
    ],
    "type": "script_exception",
    "reason": "compile error",
    "script_stack": [
      "{\"source\":\"Math.log(_scor ...",
      "^---- HERE"
    ],
    "script": "{\"source\":\"Math.log(_score * 2) + params.my_modifier\"}",
    "lang": "painless",
    "caused_by": {
      "type": "illegal_argument_exception",
      "reason": "invalid sequence of tokens near ['{'].",
      "caused_by": {
        "type": "no_viable_alt_exception",
        "reason": null
      }
    }
  },
  "status": 500
}

I couldn't figure out the correct syntax to make it work with the lang in the URL.

honzakral · 2017-07-24T13:47:07Z

This is the format that is accepted: curl -X PUT http://localhost:9200/_scripts/painless/answer -d '{"script": "return params.a + 42"}'

GlenRSmith · 2017-10-11T20:43:25Z

jenkins, please test this

GlenRSmith · 2017-10-11T22:30:32Z

tl;dr:

21:14:40 > Throwable #1: java.lang.RuntimeException: Failure at [lang_groovy/16_update2:7]: unable to find matching rest path for api [put_script] and path params {lang=1}

details:

21:14:40 Suite: org.elasticsearch.script.groovy.LangGroovyClientYamlTestSuiteIT
21:14:40   1> [2017-10-11T15:14:35,351][INFO ][o.e.s.g.LangGroovyClientYamlTestSuiteIT] [test {yaml=lang_groovy/10_basic/Groovy loaded}]: before test
21:14:40   1> [2017-10-11T15:14:35,368][INFO ][o.e.s.g.LangGroovyClientYamlTestSuiteIT] initializing REST clients against [http://[::1]:37511]
21:14:40   1> [2017-10-11T15:14:36,185][INFO ][o.e.s.g.LangGroovyClientYamlTestSuiteIT] initializing yaml client, minimum es version: [5.6.0] master version: [5.6.0] hosts: [http://[::1]:37511]
21:14:40   1> [2017-10-11T15:14:36,398][INFO ][o.e.s.g.LangGroovyClientYamlTestSuiteIT] [test {yaml=lang_groovy/10_basic/Groovy loaded}]: after test
21:14:40   1> [2017-10-11T15:14:36,428][INFO ][o.e.s.g.LangGroovyClientYamlTestSuiteIT] [test {yaml=lang_groovy/30_compile_limit/circuit breaking with too many scripts}]: before test
21:14:40   1> [2017-10-11T15:14:38,259][WARN ][o.e.c.RestClient         ] request [POST http://[::1]:37511/test/test/_search?error_trace=true] returned 1 warnings: [299 Elasticsearch-5.6.0-SNAPSHOT-4740ac2 "[groovy] scripts are deprecated, use [painless] scripts instead" "Wed, 11 Oct 2017 21:14:38 GMT"]
21:14:40   2> REPRODUCE WITH: gradle :modules:lang-groovy:integTestRunner -Dtests.seed=CCD146F95F1FB952 -Dtests.class=org.elasticsearch.script.groovy.LangGroovyClientYamlTestSuiteIT -Dtests.method="test {yaml=lang_groovy/16_update2/Stored script}" -Dtests.security.manager=true -Dtests.locale=sr-Latn-RS -Dtests.timezone=America/Managua
21:14:40   1> [2017-10-11T15:14:38,429][INFO ][o.e.s.g.LangGroovyClientYamlTestSuiteIT] [test {yaml=lang_groovy/30_compile_limit/circuit breaking with too many scripts}]: after test
21:14:40   1> [2017-10-11T15:14:38,436][INFO ][o.e.s.g.LangGroovyClientYamlTestSuiteIT] [test {yaml=lang_groovy/30_compile_limit/no bad settings}]: before test
21:14:40   1> [2017-10-11T15:14:39,569][INFO ][o.e.s.g.LangGroovyClientYamlTestSuiteIT] [test {yaml=lang_groovy/30_compile_limit/no bad settings}]: after test
21:14:40   1> [2017-10-11T15:14:39,574][INFO ][o.e.s.g.LangGroovyClientYamlTestSuiteIT] [test {yaml=lang_groovy/16_update2/Stored script}]: before test
21:14:40   1> [2017-10-11T15:14:39,590][INFO ][o.e.s.g.LangGroovyClientYamlTestSuiteIT] [test {yaml=lang_groovy/16_update2/Stored script}]: after test
21:14:40   1> [2017-10-11T15:14:39,593][INFO ][o.e.s.g.LangGroovyClientYamlTestSuiteIT] Stash dump on failure [{
21:14:40   1>   "stash" : {
21:14:40   1>     "body" : null
21:14:40   1>   }
21:14:40   1> }]
21:14:40 ERROR   0.03s | LangGroovyClientYamlTestSuiteIT.test {yaml=lang_groovy/16_update2/Stored script} <<< FAILURES!
21:14:40    > Throwable #1: java.lang.RuntimeException: Failure at [lang_groovy/16_update2:7]: unable to find matching rest path for api [put_script] and path params {lang=1}
21:14:40    > 	at __randomizedtesting.SeedInfo.seed([CCD146F95F1FB952:44857923F1E3D4AA]:0)
21:14:40    > 	at org.elasticsearch.test.rest.yaml.ESClientYamlSuiteTestCase.executeSection(ESClientYamlSuiteTestCase.java:343)
21:14:40    > 	at org.elasticsearch.test.rest.yaml.ESClientYamlSuiteTestCase.test(ESClientYamlSuiteTestCase.java:325)
21:14:40    > 	at java.lang.Thread.run(Thread.java:748)
21:14:40    > Caused by: java.lang.IllegalArgumentException: unable to find matching rest path for api [put_script] and path params {lang=1}
21:14:40    > 	at org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestApi.getFinalPaths(ClientYamlSuiteRestApi.java:140)
21:14:40    > 	at org.elasticsearch.test.rest.yaml.ClientYamlTestClient.callApi(ClientYamlTestClient.java:141)
21:14:40    > 	at org.elasticsearch.test.rest.yaml.ClientYamlTestExecutionContext.callApiInternal(ClientYamlTestExecutionContext.java:161)
21:14:40    > 	at org.elasticsearch.test.rest.yaml.ClientYamlTestExecutionContext.callApi(ClientYamlTestExecutionContext.java:93)
21:14:40    > 	at org.elasticsearch.test.rest.yaml.section.DoSection.execute(DoSection.java:221)
21:14:40    > 	at org.elasticsearch.test.rest.yaml.ESClientYamlSuiteTestCase.executeSection(ESClientYamlSuiteTestCase.java:341)
21:14:40    > 	... 38 more
21:14:40   1> [2017-10-11T15:14:39,605][INFO ][o.e.s.g.LangGroovyClientYamlTestSuiteIT] [test {yaml=lang_groovy/90_missing/Missing document (script)}]: before test
21:14:40   1> [2017-10-11T15:14:39,753][INFO ][o.e.s.g.LangGroovyClientYamlTestSuiteIT] [test {yaml=lang_groovy/90_missing/Missing document (script)}]: after test
21:14:40   1> [2017-10-11T15:14:39,759][INFO ][o.e.s.g.LangGroovyClientYamlTestSuiteIT] [test {yaml=lang_groovy/15_update/Update Script}]: before test
21:14:40   1> [2017-10-11T15:14:40,050][WARN ][o.e.c.RestClient         ] request [POST http://[::1]:37511/test_1/test/1/_update?error_trace=true] returned 1 warnings: [299 Elasticsearch-5.6.0-SNAPSHOT-4740ac2 "[groovy] scripts are deprecated, use [painless] scripts instead" "Wed, 11 Oct 2017 21:14:39 GMT"]
21:14:40   1> [2017-10-11T15:14:40,125][WARN ][o.e.c.RestClient         ] request [POST http://[::1]:37511/test_1/test/1/_update?error_trace=true] returned 1 warnings: [299 Elasticsearch-5.6.0-SNAPSHOT-4740ac2 "[groovy] scripts are deprecated, use [painless] scripts instead" "Wed, 11 Oct 2017 21:14:40 GMT"]
21:14:40   1> [2017-10-11T15:14:40,198][INFO ][o.e.s.g.LangGroovyClientYamlTestSuiteIT] [test {yaml=lang_groovy/15_update/Update Script}]: after test
21:14:40   1> [2017-10-11T15:14:40,203][INFO ][o.e.s.g.LangGroovyClientYamlTestSuiteIT] [test {yaml=lang_groovy/25_script_upsert/Script upsert}]: before test
21:14:40   1> [2017-10-11T15:14:40,385][WARN ][o.e.c.RestClient         ] request [POST http://[::1]:37511/test_1/test/1/_update?error_trace=true] returned 1 warnings: [299 Elasticsearch-5.6.0-SNAPSHOT-4740ac2 "[groovy] scripts are deprecated, use [painless] scripts instead" "Wed, 11 Oct 2017 21:14:40 GMT"]
21:14:40   1> [2017-10-11T15:14:40,421][WARN ][o.e.c.RestClient         ] request [POST http://[::1]:37511/test_1/test/2/_update?error_trace=true] returned 1 warnings: [299 Elasticsearch-5.6.0-SNAPSHOT-4740ac2 "[groovy] scripts are deprecated, use [painless] scripts instead" "Wed, 11 Oct 2017 21:14:40 GMT"]
21:14:40   1> [2017-10-11T15:14:40,486][INFO ][o.e.s.g.LangGroovyClientYamlTestSuiteIT] [test {yaml=lang_groovy/25_script_upsert/Script upsert}]: after test
21:14:40   1> [2017-10-11T15:14:40,490][INFO ][o.e.s.g.LangGroovyClientYamlTestSuiteIT] [test {yaml=lang_groovy/90_missing/Missing document (partial doc)}]: before test
21:14:40   1> [2017-10-11T15:14:40,647][INFO ][o.e.s.g.LangGroovyClientYamlTestSuiteIT] [test {yaml=lang_groovy/90_missing/Missing document (partial doc)}]: after test
21:14:40   2> NOTE: leaving temporary files on disk at: /var/lib/jenkins/workspace/elastic+elasticsearch+pull-request/modules/lang-groovy/build/testrun/integTestRunner/J0/temp/org.elasticsearch.script.groovy.LangGroovyClientYamlTestSuiteIT_CCD146F95F1FB952-001
21:14:40   2> NOTE: test params are: codec=Asserting(Lucene62): {}, docValues:{}, maxPointsInLeafNode=1567, maxMBSortInHeap=7.6583382988176005, sim=RandomSimilarity(queryNorm=true,coord=crazy): {}, locale=sr-Latn-RS, timezone=America/Managua
21:14:40   2> NOTE: Linux 4.4.0-1016-aws amd64/Oracle Corporation 1.8.0_131 (64-bit)/cpus=4,threads=1,free=447298560,total=514850816
21:14:40   2> NOTE: All tests run in this JVM: [LangGroovyClientYamlTestSuiteIT]
21:14:40 Completed [1/1] in 5.66s, 8 tests, 1 error <<< FAILURES!
21:14:40 
21:14:40 [ant:junit4] JVM J0: stderr was not empty, see: /var/lib/jenkins/workspace/elastic+elasticsearch+pull-request/modules/lang-groovy/build/testrun/integTestRunner/temp/junit4-J0-20171011_211432_4181071138114115910995.syserr
21:14:40 Tests with failures:
21:14:40   - org.elasticsearch.script.groovy.LangGroovyClientYamlTestSuiteIT.test {yaml=lang_groovy/16_update2/Stored script}
21:14:40 
21:14:40 Slow Tests Summary:
21:14:40   5.66s | org.elasticsearch.script.groovy.LangGroovyClientYamlTestSuiteIT
21:14:40 
21:14:40 [ant:junit4] JVM J0:     0.26 ..     8.55 =     8.29s
21:14:40 [ant:junit4] Execution time total: 8.55 sec.
21:14:40 [ant:junit4] Tests summary: 1 suite, 8 tests, 1 error
21:14:40 :modules:lang-groovy:integTestRunner FAILED
21:14:40 
21:14:40 Cluster modules_lang-groovy_integTestCluster - node 0 log excerpt:
21:14:40 (full log at /var/lib/jenkins/workspace/elastic+elasticsearch+pull-request/modules/lang-groovy/build/cluster/integTestCluster node0/elasticsearch-5.6.0-SNAPSHOT/logs/modules_lang-groovy_integTestCluster.log)
21:14:40 -----------------------------------------

s1monw · 2018-11-15T20:12:06Z

@honzakral is this still relevant?

honzakral · 2018-11-15T20:35:18Z

It is not, in master and 6.x the API is updated and no longer suffers from this issue. Closing

Sync the API definition for put_script with the docs

4740ac2

This was referenced Jul 15, 2017

put_script wrong url elastic/elasticsearch-py#619

Closed

Rest Tests for Painless do not respect the API spec #25737

Closed

honzakral requested a review from jdconrad July 15, 2017 15:15

jdconrad added v5.6.0 :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >docs General docs changes labels Jul 17, 2017

clintongormley added v6.1.0 and removed v5.6.0 labels Jul 25, 2017

martijnvg closed this Aug 2, 2017

honzakral changed the base branch from 5.x to 5.6 August 3, 2017 07:03

honzakral reopened this Aug 3, 2017

lcawl added v6.2.0 and removed v6.1.0 labels Dec 12, 2017

colings86 added v6.3.0 and removed v6.2.0 labels Jan 22, 2018

bleskes added v6.3.1 v6.4.0 and removed v6.3.0 v6.3.1 labels Apr 26, 2018

lcawl added the v6.4.1 label Aug 23, 2018

lcawl removed the v6.4.0 label Aug 23, 2018

honzakral closed this Nov 15, 2018

honzakral deleted the 5.x branch November 15, 2018 20:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sync the API definition for put_script with the docs#25736

Sync the API definition for put_script with the docs#25736
honzakral wants to merge 1 commit intoelastic:5.6from
honzakral:5.x

honzakral commented Jul 15, 2017

Uh oh!

jdconrad commented Jul 17, 2017

Uh oh!

martijnvg commented Jul 20, 2017

Uh oh!

jdconrad commented Jul 20, 2017

Uh oh!

honzakral commented Jul 21, 2017

Uh oh!

rjernst commented Jul 21, 2017

Uh oh!

clintongormley commented Jul 24, 2017

Uh oh!

honzakral commented Jul 24, 2017

Uh oh!

GlenRSmith commented Oct 11, 2017

Uh oh!

GlenRSmith commented Oct 11, 2017

Uh oh!

s1monw commented Nov 15, 2018

Uh oh!

honzakral commented Nov 15, 2018 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

Conversation

honzakral commented Jul 15, 2017

Uh oh!

jdconrad commented Jul 17, 2017

Uh oh!

martijnvg commented Jul 20, 2017

Uh oh!

jdconrad commented Jul 20, 2017

Uh oh!

honzakral commented Jul 21, 2017

Uh oh!

rjernst commented Jul 21, 2017

Uh oh!

clintongormley commented Jul 24, 2017

Uh oh!

honzakral commented Jul 24, 2017

Uh oh!

GlenRSmith commented Oct 11, 2017

Uh oh!

GlenRSmith commented Oct 11, 2017

Uh oh!

s1monw commented Nov 15, 2018

Uh oh!

honzakral commented Nov 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

honzakral commented Nov 15, 2018 •

edited

Loading