Add "remove a tag" script logic as an example #32556

JeffSaxeVA · 2018-08-01T16:55:24Z

It took me quite a while of online searching and experimenting to realize the function-call asymmetry in the Add versus Remove from a list, like the "tags" list! I realize we cannot give examples for every single thing the user wants to do in Painless, but this is such a common use case (removing a tag from a single doc, or from a set of docs with Update-By-Query) that I believe it ought to be demonstrated immediately after the "add a tag" example. We have an example of removing an entire document field, but not removing one element of a list (a multi-valued field).

Also, a minor grammar fix: I have added an apostrophe to the word "its" in the accompanying text of the example just above.

It took me quite a while of online searching and experimenting to realize the function-call asymmetry in the Add versus Remove from a list, like the "tags" list! I realize we cannot give examples for every single thing the user wants to do in Painless, but this is such a common use case (removing a tag from a single doc, or from a set of docs with Update-By-Query) that I believe it ought to be demonstrated immediately after the "add a tag" example. We have an example of removing an entire document field, but not removing one element of a list (a multi-valued field). Also, a minor grammar fix: I have added an apostrophe to the word "its" in the accompanying text of the example just above.

elasticmachine · 2018-08-02T12:57:10Z

Pinging @elastic/es-core-infra

JeffSaxeVA · 2018-08-13T21:39:42Z

Hi! This was my first pull request, so I don't know if I did it properly, but I believe I did. It's not code, merely documentation, so it might not be as important as real code, but I do think its inclusion would help other users who were in the same boat as I was. I respectfully submit it for your consideration to add to the docs.

Thanks!

jdconrad · 2018-08-14T18:13:19Z

@JeffSaxeVA Thank you for the contribution. This looks good to me.

jdconrad · 2018-08-14T18:15:13Z

@elasticmachine test this please

jdconrad

Will commit once it passes CI.

jdconrad · 2018-08-14T19:47:02Z

@elasticmachine test this please

jdconrad · 2018-08-14T22:15:53Z

@JeffSaxeVA Unfortunately the docs tests produced an error with this commit. You should be able to run ../gradlew check from <your-es-repo-path>/docs to reproduce the error locally. I'm wondering if this a syncing issue with master on a second glance. Would you please sync with master and push, and I will re-run the tests. Thanks in advance!

I'm seeing the following error:

1> [2018-08-15T03:26:37,578][INFO ][o.e.s.DocsClientYamlTestSuiteIT] [test] [yaml=reference/docs/update/line_19] before test
20:26:42   1> [2018-08-15T03:26:37,823][INFO ][o.e.s.DocsClientYamlTestSuiteIT] [test] Stash dump on test failure [{
20:26:42   1>   "stash" : {
20:26:42   1>     "body" : {
20:26:42   1>       "_index" : "test",
20:26:42   1>       "_type" : "_doc",
20:26:42   1>       "_id" : "1",
20:26:42   1>       "_version" : 7,
20:26:42   1>       "result" : "noop",
20:26:42   1>       "_shards" : {
20:26:42   1>         "total" : 0,
20:26:42   1>         "successful" : 0,
20:26:42   1>         "failed" : 0
20:26:42   1>       }
20:26:42   1>     }
20:26:42   1>   }
20:26:42   1> }]
20:26:42   1> [2018-08-15T03:26:37,868][INFO ][o.e.s.DocsClientYamlTestSuiteIT] [test] [yaml=reference/docs/update/line_19] after test
20:26:42   1> [2018-08-15T03:26:37,869][ERROR][o.e.s.DocsClientYamlTestSuiteIT] [test] This failing test was generated by documentation starting at reference/docs/update.asciidoc:line_19. It may include many snippets. See docs/README.asciidoc for an explanation of test generation.
20:26:42 FAILURE 0.30s | DocsClientYamlTestSuiteIT.test {yaml=reference/docs/update/line_19} <<< FAILURES!
20:26:42    > Throwable #1: java.lang.AssertionError: Failure at [reference/docs/update:120]: $body didn't match expected value:
20:26:42    >                          $body: 
20:26:42    >                              _id: same [1]
20:26:42    >                           _index: same [test]
20:26:42    >                          _shards: 
20:26:42    >                             failed: same [0]
20:26:42    >                         successful: same [0]
20:26:42    >                              total: same [0]
20:26:42    >                            _type: same [_doc]
20:26:42    >                         _version: expected [6] but was [7]
20:26:42    >                           result: same [noop]
20:26:42    > 	at __randomizedtesting.SeedInfo.seed([EC68ABD4E120AEA1:643C940E4FDCC359]:0)
20:26:42    > 	at org.elasticsearch.test.rest.yaml.ESClientYamlSuiteTestCase.executeSection(ESClientYamlSuiteTestCase.java:395)
20:26:42    > 	at org.elasticsearch.test.rest.yaml.ESClientYamlSuiteTestCase.test(ESClientYamlSuiteTestCase.java:372)
20:26:42    > 	at java.lang.Thread.run(Thread.java:748)
20:26:42    > Caused by: java.lang.AssertionError: $body didn't match expected value:
20:26:42    >                          $body: 
20:26:42    >                              _id: same [1]
20:26:42    >                           _index: same [test]
20:26:42    >                          _shards: 
20:26:42    >                             failed: same [0]
20:26:42    >                         successful: same [0]
20:26:42    >                              total: same [0]
20:26:42    >                            _type: same [_doc]
20:26:42    >                         _version: expected [6] but was [7]
20:26:42    >                           result: same [noop]
20:26:42    > 	at org.elasticsearch.test.rest.yaml.section.MatchAssertion.doAssert(MatchAssertion.java:87)
20:26:42    > 	at org.elasticsearch.test.rest.yaml.section.Assertion.execute(Assertion.java:76)
20:26:42    > 	at org.elasticsearch.test.rest.yaml.ESClientYamlSuiteTestCase.executeSection(ESClientYamlSuiteTestCase.java:388)
20:26:42    > 	... 37 more
20:26:42   1> [2018-08-15T03:26:37,894][INFO ][o.e.s.DocsClientYamlTestSuiteIT] [test] [yaml=reference/aggregations/bucket/datehistogram-aggregation/line_411] before test

JeffSaxeVA · 2018-08-15T14:21:55Z

Wow. I didn't realize there was such a sophisticated CI / test suite behind even the documentation! I didn't pull down an entire copy of the master to my local workstation using git; I merely edited the one file in question here, right inside the github portal, directly creating a check-in patch and a pull request right on the site. So I'm not sure I can easily "sync with master and push".

But I'm looking at the gradle output that you quoted, and it looks like a spurious failure. Each of the examples is being executed against a sample index called "test", trying to modify a document type "_doc" with unique ID "1". Most of the examples will actually have some effect (incrementing a counter, or adding a tag), so in each case, it would store back a new document with that ID, with new content and with an incremented _version. In fact if my new example code is executed serially right after the previous example of adding a tag "blue", then it would correctly remove that tag "blue", so the new doc would be different.

But if each example is tested individually in a vacuum, with a fresh environment and no preexisting "test" index, then my example will test for the existence of a "blue" tag, discover that there isn't one, and then come up with no modifications to the source, so I guess it would turn into a "noop" update. But then I don't know why the _version would be incremented from 6 to 7, but even if it is, that would be harmless. Maybe that's it... is the gradle test written so that a "noop" result should always imply that the _version number stays the same?

I am not sure how I can fix this problem. I'm pretty sure my code snippet works (used it myself to bulk-repair some faulty documents at my job), so I'm not sure how I can convince gradle that it doesn't have an error. Hmm.

javanna · 2018-08-16T09:22:21Z

retest this please

javanna · 2018-08-16T09:22:51Z

I have merged master in and ran tests again, fingers crossed ;)

javanna · 2018-08-16T11:02:39Z

retest this please

javanna · 2018-08-16T13:03:22Z

retest this please

javanna · 2018-08-16T13:06:33Z

hi @JeffSaxeVA I could verify that the error was caused by your change. A snippet down below was checking that the version of the document was set to 6, but with your additional update it is now 7. I changed that and also wrapped the new added lines to 80 character per line and pushed to your remote. I triggered a new build, which should be green.

JeffSaxeVA · 2018-08-16T14:03:47Z

Ah, thank you, @javanna! I now see how it works... TEST, TEST, then a TESTRESPONSE that is checked for correctness. That's a good way to make documentation with code snippets and ensure the code works.

Thanks for your generosity in repairing and reformatting my contribution, and my apologies for not discovering the 6 and 7 a bit further down within the same document.

javanna · 2018-08-16T16:50:40Z

the current failure is unrelated, I will trigger a new build once we fixed that upstream.

javanna · 2018-08-16T18:28:08Z

retest this please

javanna · 2018-08-17T08:56:17Z

thanks @JeffSaxeVA !

It took me quite a while of online searching and experimenting to realize the function-call asymmetry in the Add versus Remove from a list, like the "tags" list! I realize we cannot give examples for every single thing the user wants to do in Painless, but this is such a common use case (removing a tag from a single doc, or from a set of docs with Update-By-Query) that I believe it ought to be demonstrated immediately after the "add a tag" example. We have an example of removing an entire document field, but not removing one element of a list (a multi-valued field). Also, a minor grammar fix: I have added an apostrophe to the word "its" in the accompanying text of the example just above.

* 6.x: (42 commits) [DOCS] Splits the users API documentation into multiple pages (#32825) [DOCS] Splits the token APIs into separate pages (#32865) [DOCS] Creates redirects for role management APIs page Bypassing failing test PainlessDomainSplitIT#testHRDSplit (#32966) TEST: Mute testRetentionPolicyChangeDuringRecovery [DOCS] Fixes more broken links to role management APIs [Docs] Tweaks and fixes to rollup docs [DOCS] Fixes links to role management APIs [ML][TEST] Fix BasicRenormalizationIT after adding multibucket feature [DOCS] Splits the roles API documentation into multiple pages (#32794) [TEST] Run pre 6.4 nodes in non-FIPS JVMs (#32901) Remove assertion in testDocStats on deletedDocs counter (#32914) [ML] fix updating opened jobs scheduled events (#31651) (#32881) Enable FIPS140LicenseBootstrapCheck (#32903) HLRC: Move ML request converters into their own class (#32906) [DOCS] Update getting-started.asciidoc (#29518) Fix allowed value for HighlighterBuilder encoder in javadocs (#32780) [DOCS] Add "remove a tag" script logic as an example (#32556) RFC: Test that example plugins build stand-alone (#32235) Security: remove put privilege API (#32879) ...

JeffSaxeVA · 2018-08-21T14:52:47Z

Thanks for all your help, @javanna and @jdconrad !

colings86 added >docs General docs changes :Data Management/Indices APIs APIs to create and manage indices and templates labels Aug 2, 2018

jdconrad self-requested a review August 14, 2018 18:12

jdconrad added v7.0.0 v6.5.0 labels Aug 14, 2018

jdconrad approved these changes Aug 14, 2018

View reviewed changes

Merge branch 'master' into patch-2

5dbd907

Merge branch 'master' into patch-2

6f4824f

fix test problem and wrap new lines

9f4af32

Merge branch 'master' into patch-2

cd7295f

javanna merged commit efdad7d into elastic:master Aug 17, 2018

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add "remove a tag" script logic as an example #32556

Add "remove a tag" script logic as an example #32556

JeffSaxeVA commented Aug 1, 2018

elasticmachine commented Aug 2, 2018

JeffSaxeVA commented Aug 13, 2018

jdconrad commented Aug 14, 2018

jdconrad commented Aug 14, 2018

jdconrad left a comment

jdconrad commented Aug 14, 2018

jdconrad commented Aug 14, 2018 •

edited

Loading

JeffSaxeVA commented Aug 15, 2018

javanna commented Aug 16, 2018

javanna commented Aug 16, 2018

javanna commented Aug 16, 2018

javanna commented Aug 16, 2018

javanna commented Aug 16, 2018

JeffSaxeVA commented Aug 16, 2018

javanna commented Aug 16, 2018

javanna commented Aug 16, 2018

javanna commented Aug 17, 2018

JeffSaxeVA commented Aug 21, 2018

Add "remove a tag" script logic as an example #32556

Add "remove a tag" script logic as an example #32556

Conversation

JeffSaxeVA commented Aug 1, 2018

elasticmachine commented Aug 2, 2018

JeffSaxeVA commented Aug 13, 2018

jdconrad commented Aug 14, 2018

jdconrad commented Aug 14, 2018

jdconrad left a comment

Choose a reason for hiding this comment

jdconrad commented Aug 14, 2018

jdconrad commented Aug 14, 2018 • edited Loading

JeffSaxeVA commented Aug 15, 2018

javanna commented Aug 16, 2018

javanna commented Aug 16, 2018

javanna commented Aug 16, 2018

javanna commented Aug 16, 2018

javanna commented Aug 16, 2018

JeffSaxeVA commented Aug 16, 2018

javanna commented Aug 16, 2018

javanna commented Aug 16, 2018

javanna commented Aug 17, 2018

JeffSaxeVA commented Aug 21, 2018

jdconrad commented Aug 14, 2018 •

edited

Loading