Rename 'fields' property to 'stored_fields' in routes.js test to adhere to new ES msearch API. #7542

cjcenizal · 2016-06-24T03:40:32Z

Issues addressed:

Kibana is having an issue with a change to ES: Rename fields parameter to stored_fields #7541
This also addresses the error that occurs when you run esvm:test:

1) plugins/elasticsearch routes POST /elasticsearch/_msearch?timeout=0&ignore_unavailable=true&preference=1429577952339 should should return 200:
     Error: expected 400 to equal 200
      at Assertion.assert (node_modules/expect.js/index.js:96:13)
      at Assertion.be.Assertion.equal (node_modules/expect.js/index.js:216:10)
      at Assertion.(anonymous function) [as be] (node_modules/expect.js/index.js:69:24)
      at src/plugins/elasticsearch/lib/__tests__/routes.js:42:41
      at node_modules/hapi/lib/connection.js:281:16
      at node_modules/hapi/node_modules/shot/lib/index.js:206:13

@ycombinator identified this change to ES as the culprit: elastic/elasticsearch@2f46f53

ycombinator · 2016-06-24T10:25:37Z

@cjcenizal Thanks for creating this PR. I suspect this change is needed in several other places as well. I'm not sure of the extent but it probably makes sense to group the same change (in several places) into this PR.

P.S. I realize there is a build failure for an unrelated, different ES change with _timestamp. I would keep any changes related to that out of this PR.

Bargs · 2016-06-24T13:08:25Z

Looks like this is causing an issue in Kibana itself as well #7541

jbudz · 2016-06-24T15:41:04Z

This is going to require/be dependent on a new elasticsearch client library publish.

uboness · 2016-06-24T15:43:22Z

src/plugins/elasticsearch/lib/__tests__/routes.js

@@ -105,7 +105,7 @@ describe('plugins/elasticsearch', function () {
    testRoute({
      method: 'POST',
      url: '/elasticsearch/_msearch?timeout=0&ignore_unavailable=true&preference=1429577952339',
-      payload: '{"index":"logstash-2015.04.21","ignore_unavailable":true}\n{"size":500,"sort":{"@timestamp":"desc"},"query":{"bool":{"must":[{"query_string":{"analyze_wildcard":true,"query":"*"}},{"bool":{"must":[{"range":{"@timestamp":{"gte":1429577068175,"lte":1429577968175}}}],"must_not":[]}}],"must_not":[]}},"highlight":{"pre_tags":["@kibana-highlighted-field@"],"post_tags":["@/kibana-highlighted-field@"],"fields":{"*":{}}},"aggs":{"2":{"date_histogram":{"field":"@timestamp","interval":"30s","min_doc_count":0,"extended_bounds":{"min":1429577068175,"max":1429577968175}}}},"fields":["*","_source"],"script_fields":{},"fielddata_fields":["timestamp_offset","@timestamp","utc_time"]}\n' // eslint-disable-line max-len
+      payload: '{"index":"logstash-2015.04.21","ignore_unavailable":true}\n{"size":500,"sort":{"@timestamp":"desc"},"query":{"bool":{"must":[{"query_string":{"analyze_wildcard":true,"query":"*"}},{"bool":{"must":[{"range":{"@timestamp":{"gte":1429577068175,"lte":1429577968175}}}],"must_not":[]}}],"must_not":[]}},"highlight":{"pre_tags":["@kibana-highlighted-field@"],"post_tags":["@/kibana-highlighted-field@"],"fields":{"*":{}}},"aggs":{"2":{"date_histogram":{"field":"@timestamp","interval":"30s","min_doc_count":0,"extended_bounds":{"min":1429577068175,"max":1429577968175}}}},"stored_fields":["*","_source"],"script_fields":{},"fielddata_fields":["timestamp_offset","@timestamp","utc_time"]}\n' // eslint-disable-line max-len


what's the point in having stored_fields: [ "*", "_source"] ? we can just leave out this parameter

Per Zoom discussion, we'll make a list of all the places this type of reference exists, and then make a separate PR for removing them. CC @jbudz

If you want to return the source regardless, then use "_source": true, so to return all stored fields AND the source, use:

"stored_fields": "*", "_source": true

uboness · 2016-06-24T16:09:25Z

Looks like this is causing an issue in Kibana itself as well #7541

indeed... so far I also found:

jbudz · 2016-06-24T16:43:56Z

#7546 should fix the functional tests _timestamp issue

jbudz · 2016-06-24T23:01:41Z

jenkins, test it

jbudz · 2016-06-24T23:09:29Z

...public/management/sections/indices/add_data_steps/pattern_review_step/pattern_review_step.js

@@ -77,7 +77,7 @@ modules.get('apps/management')
        _.defaults(this.indexPattern, {
          id: this.patternInput.defaultValue,
          title: 'filebeat-*',
-          fields: _(sampleFields)
+          stored_fields: _(sampleFields)


apologies, this one should still be fields

spalger · 2016-06-25T00:42:12Z

LGTM

uboness · 2016-06-25T00:48:54Z

src/plugins/kibana/public/dashboard/services/saved_dashboards.js

@@ -69,7 +69,7 @@ module.service('savedDashboards', function (Promise, SavedDashboard, kbnIndex, e
        query: {
          simple_query_string: {
            query: searchString + '*',
-            fields: ['title^3', 'description'],
+            stored_fields: ['title^3', 'description'],


hmm... afaik we didn't change the simple_query_string query... are you sure this is working?

Seems dubious - we search indexed fields not stored ones so either way is odd naming.

This should definitely be fields, not stored_fields

cjcenizal · 2016-06-27T23:26:22Z

TODO: We can merge this after elasticsearch un-reverts their "fields -> stored_fields" change. Then we can squash these commits, and rebase master onto this branch to bring everything up to date. Then we should be good to merge.

epixa · 2016-07-05T13:16:54Z

Merging this would happen after all of those things :)

cjcenizal · 2016-07-05T13:54:09Z

Note: the "fields -> stored_fields" change has been unreverted in ES: elastic/elasticsearch@afe99fc

…ch API. - Upgrade elasticsearch client to 12.0.0-rc4.

jbudz · 2016-07-05T15:10:34Z

rebase LGTM

ycombinator · 2016-07-05T15:27:36Z

I'm seeing an issue in the doc viewer in the Discover view. If I expand on one of the rows, both the Table and JSON tabs are empty:

There are no errors in the developer console.

I'm not entirely sure this issue is a result of the changes in this PR. Unfortunately master is completely broken without this PR so I can't compare against that. I'm comparing against 5.0.0-alpha4 now...

jbudz · 2016-07-05T15:31:13Z

@ycombinator you'll want to use es master for this, this should fix the break

ycombinator · 2016-07-05T15:32:32Z

@jbudz I am using ES master, specifically this commit SHA: elastic/elasticsearch@dec620c

jbudz · 2016-07-05T15:33:10Z

Ah I misunderstood, got it.

ycombinator · 2016-07-05T15:35:41Z

BTW, just confirmed that Kibana 5.0.0-alpha4 does not show the empty table or JSON issue I reported in #7542 (comment).

jbudz · 2016-07-05T17:21:02Z

Opened #7628

ycombinator · 2016-07-05T17:33:32Z

Thanks @jbudz. Given that the issue I reported is unrelated to this PR, I'll continue reviewing this PR.

ycombinator · 2016-07-05T17:58:10Z

LGTM.

…d-fields Rename 'fields' property to 'stored_fields' in routes.js test to adhere to new ES msearch API. Former-commit-id: 5588983

Bargs mentioned this pull request Jun 24, 2016

Rename fields parameter to stored_fields #7541

Closed

cjcenizal changed the title ~~Rename 'fields' property to 'stored_fields' in routes.js test to adhere to new ES msearch API.~~ WIP: Rename 'fields' property to 'stored_fields' in routes.js test to adhere to new ES msearch API. Jun 24, 2016

uboness reviewed Jun 24, 2016
View reviewed changes

cjcenizal force-pushed the bug/msearch-test-stored-fields branch from 5f6deea to 0b02e38 Compare June 24, 2016 16:45

cjcenizal added the blocked label Jun 24, 2016

jbudz added blocker release_note:breaking v5.0.0-alpha4 labels Jun 24, 2016

cjcenizal force-pushed the bug/msearch-test-stored-fields branch from 7ee3e42 to 82980d2 Compare June 24, 2016 22:39

cjcenizal changed the title ~~WIP: Rename 'fields' property to 'stored_fields' in routes.js test to adhere to new ES msearch API.~~ Rename 'fields' property to 'stored_fields' in routes.js test to adhere to new ES msearch API. Jun 24, 2016

jbudz added review and removed blocked labels Jun 24, 2016

jbudz reviewed Jun 24, 2016
View reviewed changes

jbudz added the v5.0.0-beta1 label Jun 24, 2016

spalger assigned spalger and cjcenizal Jun 24, 2016

cjcenizal force-pushed the bug/msearch-test-stored-fields branch from 685ef4c to 1169670 Compare June 25, 2016 00:04

uboness reviewed Jun 25, 2016
View reviewed changes

cjcenizal changed the title ~~Rename 'fields' property to 'stored_fields' in routes.js test to adhere to new ES msearch API.~~ [WIP]: Rename 'fields' property to 'stored_fields' in routes.js test to adhere to new ES msearch API. Jun 27, 2016

cjcenizal force-pushed the bug/msearch-test-stored-fields branch 2 times, most recently from dd0c399 to 7cbea3f Compare July 5, 2016 14:05

cjcenizal changed the title ~~[WIP]: Rename 'fields' property to 'stored_fields' in routes.js test to adhere to new ES msearch API.~~ Rename 'fields' property to 'stored_fields' in routes.js test to adhere to new ES msearch API. Jul 5, 2016

cjcenizal force-pushed the bug/msearch-test-stored-fields branch from 7cbea3f to 3c71b6e Compare July 5, 2016 14:52

Rename 'fields' property to 'stored_fields' to adhere to new ES msear…

d9cf54f

…ch API. - Upgrade elasticsearch client to 12.0.0-rc4.

cjcenizal force-pushed the bug/msearch-test-stored-fields branch from 3c71b6e to d9cf54f Compare July 5, 2016 15:02

cjcenizal assigned ycombinator Jul 5, 2016

jbudz mentioned this pull request Jul 5, 2016

Empty document tables #7628

Closed

ycombinator removed their assignment Jul 5, 2016

cjcenizal merged commit 5588983 into elastic:master Jul 5, 2016

cjcenizal deleted the bug/msearch-test-stored-fields branch July 5, 2016 18:32

jbudz mentioned this pull request Jul 6, 2016

[console autocomplete] Update fields->stored_fields, add docvalue_fields #7642

Merged

epixa added v5.0.0-alpha5 and removed v5.0.0-beta1 labels Jul 20, 2016

epixa added v5.0.0 and removed v5.0.0 labels Aug 1, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename 'fields' property to 'stored_fields' in routes.js test to adhere to new ES msearch API. #7542

Rename 'fields' property to 'stored_fields' in routes.js test to adhere to new ES msearch API. #7542

cjcenizal commented Jun 24, 2016 •

edited

Loading

ycombinator commented Jun 24, 2016 •

edited

Loading

Bargs commented Jun 24, 2016

jbudz commented Jun 24, 2016 •

edited

Loading

uboness Jun 24, 2016

cjcenizal Jun 24, 2016

clintongormley Jun 27, 2016

uboness commented Jun 24, 2016

jbudz commented Jun 24, 2016

jbudz commented Jun 24, 2016

jbudz Jun 24, 2016

spalger commented Jun 25, 2016

uboness Jun 25, 2016 •

edited

Loading

markharwood Jun 27, 2016

clintongormley Jun 27, 2016

cjcenizal commented Jun 27, 2016

epixa commented Jul 5, 2016

cjcenizal commented Jul 5, 2016

jbudz commented Jul 5, 2016

ycombinator commented Jul 5, 2016 •

edited

Loading

jbudz commented Jul 5, 2016

ycombinator commented Jul 5, 2016

jbudz commented Jul 5, 2016

ycombinator commented Jul 5, 2016

jbudz commented Jul 5, 2016

ycombinator commented Jul 5, 2016

ycombinator commented Jul 5, 2016

Rename 'fields' property to 'stored_fields' in routes.js test to adhere to new ES msearch API. #7542

Rename 'fields' property to 'stored_fields' in routes.js test to adhere to new ES msearch API. #7542

Conversation

cjcenizal commented Jun 24, 2016 • edited Loading

ycombinator commented Jun 24, 2016 • edited Loading

Bargs commented Jun 24, 2016

jbudz commented Jun 24, 2016 • edited Loading

uboness Jun 24, 2016

Choose a reason for hiding this comment

cjcenizal Jun 24, 2016

Choose a reason for hiding this comment

clintongormley Jun 27, 2016

Choose a reason for hiding this comment

uboness commented Jun 24, 2016

jbudz commented Jun 24, 2016

jbudz commented Jun 24, 2016

jbudz Jun 24, 2016

Choose a reason for hiding this comment

spalger commented Jun 25, 2016

uboness Jun 25, 2016 • edited Loading

Choose a reason for hiding this comment

markharwood Jun 27, 2016

Choose a reason for hiding this comment

clintongormley Jun 27, 2016

Choose a reason for hiding this comment

cjcenizal commented Jun 27, 2016

epixa commented Jul 5, 2016

cjcenizal commented Jul 5, 2016

jbudz commented Jul 5, 2016

ycombinator commented Jul 5, 2016 • edited Loading

jbudz commented Jul 5, 2016

ycombinator commented Jul 5, 2016

jbudz commented Jul 5, 2016

ycombinator commented Jul 5, 2016

jbudz commented Jul 5, 2016

ycombinator commented Jul 5, 2016

ycombinator commented Jul 5, 2016

cjcenizal commented Jun 24, 2016 •

edited

Loading

ycombinator commented Jun 24, 2016 •

edited

Loading

jbudz commented Jun 24, 2016 •

edited

Loading

uboness Jun 25, 2016 •

edited

Loading

ycombinator commented Jul 5, 2016 •

edited

Loading