removes vis dependency from response handlers #22583

ppisljar · 2018-08-31T12:50:01Z

removes vis dependency from response handlers
makes tabify the output of courier request handler

preparation for #19813

qa: nothing should change functionally

elasticmachine · 2018-08-31T13:08:09Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-09-13T08:34:46Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

…ult response from courier request handler

elasticmachine · 2018-09-14T06:23:13Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

ppisljar · 2018-09-14T09:50:49Z

src/core_plugins/kbn_vislib_vis_types/public/pie.js

@@ -100,6 +98,6 @@ export default function HistogramVisType(Private) {
      ])
    },
    hierarchicalData: true,
-    responseConverter: buildHierarchicalData,
+    responseHandler: 'vislib_slices',


vislib now registers two response handlers: vislib_slices and vislib_series ....
responseConverter was removed

ppisljar · 2018-09-14T09:51:47Z

src/core_plugins/table_vis/public/__tests__/_table_vis_controller.js

 import expect from 'expect.js';
 import ngMock from 'ng_mock';
 import { LegacyResponseHandlerProvider } from 'ui/vis/response_handlers/legacy';
 import { VisProvider } from 'ui/vis';
 import FixturesStubbedLogstashIndexPatternProvider from 'fixtures/stubbed_logstash_index_pattern';
 import { AppStateProvider } from 'ui/state_management/app_state';
+import { tabifyAggResponse } from '../../../../ui/public/agg_response/tabify';


in tests we used elasticsearch stubbed responses before, now we need to tabify them first.
this is far from ideal, but should be ok short term solution, as we plan to migrate our tests to the pipeline as soon as that is available.

can't we write this import as import { tabifyAggResponse } from 'ui/agg_response/tabify';?

ppisljar · 2018-09-14T09:52:35Z

src/core_plugins/tile_map/public/coordinatemap_response_handler.js


-  return function (vis, esResponse) {
-    if (lastEsResponse === esResponse) {


this is handled on visualize_data_loader level, response handler (any) will not be called if input data did not change

ppisljar · 2018-09-14T09:53:09Z

src/ui/public/vis/request_handlers/courier.js

@@ -155,20 +155,26 @@ const CourierRequestHandlerProvider = function () {

              searchSource.finalResponse = resp;

+              searchSource.tabifiedResponse = tabifyAggResponse(aggs, resp, {


courier now returns tabified data

ppisljar · 2018-09-14T09:53:27Z

src/ui/public/vis/response_handlers/legacy.js

 const LegacyResponseHandlerProvider = function () {

  return {
    name: 'legacy',
-    handler: function (vis, response) {
+    handler: function (table) {


response handlers accept only tabified data as an argument

You should also fix the typing in response_handler.d.ts to remove the vis param from it.

its already removed right ?

elasticmachine · 2018-09-14T10:10:53Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

markov00

Code LGTM. nits: about the imports: sometimes absolute paths are more readable than relative paths specially if you are importing things from different separated modules.

markov00 · 2018-09-14T10:25:36Z

src/core_plugins/table_vis/public/__tests__/_table_vis_controller.js

 import expect from 'expect.js';
 import ngMock from 'ng_mock';
 import { LegacyResponseHandlerProvider } from 'ui/vis/response_handlers/legacy';
 import { VisProvider } from 'ui/vis';
 import FixturesStubbedLogstashIndexPatternProvider from 'fixtures/stubbed_logstash_index_pattern';
 import { AppStateProvider } from 'ui/state_management/app_state';
+import { tabifyAggResponse } from '../../../../ui/public/agg_response/tabify';


can't we write this import as import { tabifyAggResponse } from 'ui/agg_response/tabify';?

markov00 · 2018-09-14T10:40:42Z

src/core_plugins/tile_map/public/coordinatemap_response_handler.js

-    lastGeoJsonResponse = convertToGeoJson(tabifiedResponse);
-
-    return lastGeoJsonResponse;
+    return convertToGeoJson(table);


what about rewriting this like

export function makeGeoJsonResponseHandler() { return convertToGeoJson; }

markov00 · 2018-09-14T10:42:40Z

src/ui/public/agg_table/__tests__/_group.js

@@ -24,13 +24,35 @@ import fixtures from 'fixtures/fake_hierarchical_data';
 import { LegacyResponseHandlerProvider } from '../../vis/response_handlers/legacy';
 import FixturesStubbedLogstashIndexPatternProvider from 'fixtures/stubbed_logstash_index_pattern';
 import { VisProvider } from '../../vis';
+import { tabifyAggResponse } from '../../agg_response/tabify';


can we use also here absolute paths from 'ui/agg_response/tabify?

nope, should use absolute paths inside the same module

markov00 · 2018-09-14T10:43:01Z

src/ui/public/agg_table/__tests__/_table.js

@@ -26,6 +26,8 @@ import sinon from 'sinon';
 import { LegacyResponseHandlerProvider } from '../../vis/response_handlers/legacy';
 import FixturesStubbedLogstashIndexPatternProvider from 'fixtures/stubbed_logstash_index_pattern';
 import { VisProvider } from '../../vis';
+import { tabifyAggResponse } from '../../agg_response/tabify';


Same as comment above

elasticmachine · 2018-09-14T11:43:10Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-09-14T13:35:54Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-09-14T13:38:52Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-09-14T16:01:55Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

ppisljar · 2018-09-17T07:24:04Z

retest

elasticmachine · 2018-09-17T07:24:06Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

ppisljar · 2018-09-17T07:24:32Z

retest

elasticmachine · 2018-09-17T09:03:52Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

ppisljar · 2018-09-17T09:04:40Z

retest

elasticmachine · 2018-09-17T10:43:44Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-09-17T13:26:28Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

timroes

Code LGTM assumign tests pass. We already should cleanup courier request handler to build the isnpector response on top of tabify, but not in this PR

elasticmachine · 2018-09-17T17:36:29Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-10-23T13:48:15Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

ppisljar added WIP Work in progress Feature:Visualizations Generic visualization features (in case no more specific feature label is available) v7.0.0 v6.5.0 labels Aug 31, 2018

ppisljar force-pushed the fix/courierTabifyJoin branch from 07e4644 to b72745a Compare September 13, 2018 08:15

remove vis dependency from response handlers and make tabify the defa…

ba7db19

…ult response from courier request handler

ppisljar force-pushed the fix/courierTabifyJoin branch from b72745a to ba7db19 Compare September 14, 2018 05:38

ppisljar removed the WIP Work in progress label Sep 14, 2018

ppisljar requested review from timroes, markov00 and lukeelmers September 14, 2018 05:39

ppisljar added the WIP Work in progress label Sep 14, 2018

fixing unit tests

5fcf791

ppisljar commented Sep 14, 2018

View reviewed changes

ppisljar added the release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. label Sep 14, 2018

markov00 approved these changes Sep 14, 2018

View reviewed changes

ppisljar added 2 commits September 14, 2018 12:53

passing parsed time range to tabify

a06da5e

updating based on marcos review

7be2e39

ppisljar requested a review from nreese September 14, 2018 10:56

ppisljar removed the WIP Work in progress label Sep 14, 2018

cleanup

c8dec37

fixing discovery

98c73e5

ppisljar force-pushed the fix/courierTabifyJoin branch from ca5b92d to 98c73e5 Compare September 14, 2018 14:39

ppisljar mentioned this pull request Sep 16, 2018

visualize canvas pipeline #23185

Closed

cleanup

3a6ba2c

only parsing timeRange if its defined

4cce197

ppisljar added review and removed WIP Work in progress labels Sep 17, 2018

cleanup

d8a9e80

timroes approved these changes Sep 17, 2018

View reviewed changes

ppisljar merged commit de345de into elastic:master Sep 17, 2018

ppisljar deleted the fix/courierTabifyJoin branch September 17, 2018 17:44

ppisljar mentioned this pull request Sep 17, 2018

[6.x] removes vis dependency from response handlers (#22583) #23244

Merged

ppisljar added a commit to ppisljar/kibana that referenced this pull request Sep 17, 2018

removes vis dependency from response handlers (elastic#22583)

9803e07

ppisljar added a commit that referenced this pull request Sep 19, 2018

removes vis dependency from response handlers (#22583) (#23244)

a93723a

ppisljar restored the fix/courierTabifyJoin branch September 26, 2018 10:02

timroes mentioned this pull request Oct 23, 2018

Visualization Request/Response handler changes #24396

Closed

timroes removed the release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. label Oct 23, 2018

timroes mentioned this pull request Nov 14, 2018

[6.4.0] $rootScope.$listen is not a function error while trying to get VisTypes list #23278

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

removes vis dependency from response handlers #22583

removes vis dependency from response handlers #22583

ppisljar commented Aug 31, 2018 •

edited by timroes

Loading

elasticmachine commented Aug 31, 2018

elasticmachine commented Sep 13, 2018

elasticmachine commented Sep 14, 2018

ppisljar Sep 14, 2018

ppisljar Sep 14, 2018

markov00 Sep 14, 2018

ppisljar Sep 14, 2018

ppisljar Sep 14, 2018

ppisljar Sep 14, 2018

timroes Sep 17, 2018

ppisljar Sep 17, 2018

elasticmachine commented Sep 14, 2018

markov00 left a comment

markov00 Sep 14, 2018

markov00 Sep 14, 2018

markov00 Sep 14, 2018

ppisljar Sep 14, 2018

markov00 Sep 14, 2018

elasticmachine commented Sep 14, 2018

elasticmachine commented Sep 14, 2018

elasticmachine commented Sep 14, 2018

elasticmachine commented Sep 14, 2018

ppisljar commented Sep 17, 2018

elasticmachine commented Sep 17, 2018

ppisljar commented Sep 17, 2018

elasticmachine commented Sep 17, 2018

ppisljar commented Sep 17, 2018

elasticmachine commented Sep 17, 2018

elasticmachine commented Sep 17, 2018

timroes left a comment

elasticmachine commented Sep 17, 2018

elasticmachine commented Oct 23, 2018


		return function (vis, esResponse) {
		if (lastEsResponse === esResponse) {

		@@ -155,20 +155,26 @@ const CourierRequestHandlerProvider = function () {

		searchSource.finalResponse = resp;

		searchSource.tabifiedResponse = tabifyAggResponse(aggs, resp, {

removes vis dependency from response handlers #22583

removes vis dependency from response handlers #22583

Conversation

ppisljar commented Aug 31, 2018 • edited by timroes Loading

elasticmachine commented Aug 31, 2018

💔 Build Failed

elasticmachine commented Sep 13, 2018

💔 Build Failed

elasticmachine commented Sep 14, 2018

💔 Build Failed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Sep 14, 2018

💔 Build Failed

markov00 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Sep 14, 2018

💔 Build Failed

elasticmachine commented Sep 14, 2018

💔 Build Failed

elasticmachine commented Sep 14, 2018

💔 Build Failed

elasticmachine commented Sep 14, 2018

💔 Build Failed

ppisljar commented Sep 17, 2018

elasticmachine commented Sep 17, 2018

💔 Build Failed

ppisljar commented Sep 17, 2018

elasticmachine commented Sep 17, 2018

💔 Build Failed

ppisljar commented Sep 17, 2018

elasticmachine commented Sep 17, 2018

💔 Build Failed

elasticmachine commented Sep 17, 2018

💚 Build Succeeded

timroes left a comment

Choose a reason for hiding this comment

elasticmachine commented Sep 17, 2018

💚 Build Succeeded

elasticmachine commented Oct 23, 2018

💔 Build Failed

ppisljar commented Aug 31, 2018 •

edited by timroes

Loading