add autoRefreshFetch event to timefilter by nreese · Pull Request #20863 · elastic/kibana

nreese · 2018-07-16T21:57:50Z

short term fix for #20813

SearchPoll emits the timefilter event autoRefreshFetch when the auto refresh interval is reached. Visualizations subscribe to this event and fetch data.

cc @timroes @stacey-gammon @markov00 @cjcenizal

spalger · 2018-07-16T23:01:00Z

src/ui/public/visualize/loader/embedded_visualize_handler.js

    this._vis.on('update', this._handleVisUpdate);
    this._vis.on('reload', this._reloadVis);
    this._uiState.on('change', this._fetchAndRender);
+    timefilter.on('autoRefreshFetch', this._fetchAndRender);


I'm not 100% sure what the forceFetch parameter does, but I think it means it won't refetch data unless the vis params has changed so we probably want to use this._reloadVis as the listener here, just like we do for the reload event on this._vis.

Yeah I would agree with Spencer here. forceFetch will actually be passed to the request handler, and it should use it to ignore every possible caching it has. In case of the courier request handler, that means, it won't check if any configuration/timeRange, etc actually changed, and instead always query again. So we will need this for auto refresh, since in case of auto refresh no configuration change, so please replace that by calling this._reloadVis.

elasticmachine · 2018-07-16T23:14:09Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

timroes · 2018-07-17T07:18:14Z

src/ui/public/visualize/loader/embedded_visualize_handler.js

    this._vis.on('update', this._handleVisUpdate);
    this._vis.on('reload', this._reloadVis);
    this._uiState.on('change', this._fetchAndRender);
+    timefilter.on('autoRefreshFetch', this._fetchAndRender);


Yeah I would agree with Spencer here. forceFetch will actually be passed to the request handler, and it should use it to ignore every possible caching it has. In case of the courier request handler, that means, it won't check if any configuration/timeRange, etc actually changed, and instead always query again. So we will need this for auto refresh, since in case of auto refresh no configuration change, so please replace that by calling this._reloadVis.

timroes · 2018-07-17T07:20:04Z

src/core_plugins/kibana/public/visualize/editor/editor.js


    timefilter.enableAutoRefreshSelector();
    $scope.$listenAndDigestAsync(timefilter, 'timeUpdate', updateTimeRange);
+    $scope.$listenAndDigestAsync(timefilter, 'autoRefreshFetch', updateTimeRange);


I think we won't need that. Since another auto refresh trigger, will never cause the time to actually change, since timefilter.getTime() will return the still stringified from/to, like "now-7d". Have you experienced any issues without that listener?

…refresh_event

elasticmachine · 2018-07-18T17:15:36Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

…refresh_event

elasticmachine · 2018-07-23T23:10:09Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

nreese · 2018-07-24T00:47:23Z

jenkins, test this

elasticmachine · 2018-07-24T02:10:03Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

timroes · 2018-07-24T07:22:05Z

Jenkins, test this

elasticmachine · 2018-07-24T08:39:37Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

timroes · 2018-07-24T08:40:23Z

Jenkins, test this

elasticmachine · 2018-07-24T10:01:58Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

ppisljar

this adds the concept of visualize reaching out to some global context and knowing about implementation details of other systems in order to work correctly.

this is something we were trying to remove (and of course broke auto refresh in the process 🥇 )

i would suggest we keep to the idea of visualize having no idea about the outside world and move the responsibility of handling autorefresh to the embedding app.

@timroes what do you think ?

timroes · 2018-07-24T12:38:01Z

@ppisljar I think the proper long term solution is to build an own service for auto refresh as Nathan discussed with Spencer (see #20813 (comment)). Though I think for a short term solution (since this will block 6.4) release and that auto refresh service should be properly design, this is a viable solution I would be willing to go with.

elasticmachine · 2018-07-24T12:47:46Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

ppisljar

LGTM as its a working solution and its a blocker and as we have a plan on how to improve it in the long run.

timroes

Code LGTM as a short term solution, but we'll still want to create a proper auto refresh service (@nreese could you perhaps open another ticket for that, so it won't be forgotten?).

Tested in Chrome Linux, auto refresh now works fine on dashboard and in visualize again.

* add autoRefreshFetch event to timefilter * replace fetchAndRender with reloadVis remove listener from editor * unsubscribe correct handler

nreese · 2018-07-24T14:23:11Z

but we'll still want to create a proper auto refresh service (@nreese could you perhaps open another ticket for that, so it won't be forgotten?).

Issue created - #21150. Please add to the ticket as needed

* add autoRefreshFetch event to timefilter * replace fetchAndRender with reloadVis remove listener from editor * unsubscribe correct handler

add autoRefreshFetch event to timefilter

d6b34d2

nreese added the WIP Work in progress label Jul 16, 2018

nreese mentioned this pull request Jul 16, 2018

Auto refresh is no longer working #20813

Closed

spalger reviewed Jul 16, 2018

View reviewed changes

timroes suggested changes Jul 17, 2018

View reviewed changes

nreese added 2 commits July 18, 2018 09:43

Merge branch 'master' of https://github.com/elastic/kibana into auto_…

6c77159

…refresh_event

replace fetchAndRender with reloadVis remove listener from editor

b62d399

nreese added 2 commits July 23, 2018 15:24

Merge branch 'master' of https://github.com/elastic/kibana into auto_…

b31cefa

…refresh_event

unsubscribe correct handler

5d05b0a

timroes requested a review from ppisljar July 24, 2018 07:18

Merge branch 'master' into auto_refresh_event

4836359

timroes added blocker v6.4.0 labels Jul 24, 2018

ppisljar requested changes Jul 24, 2018

View reviewed changes

ppisljar approved these changes Jul 24, 2018

View reviewed changes

timroes approved these changes Jul 24, 2018

View reviewed changes

nreese merged commit 6132cd9 into elastic:master Jul 24, 2018

nreese added a commit to nreese/kibana that referenced this pull request Jul 24, 2018

add autoRefreshFetch event to timefilter (elastic#20863)

c317252

* add autoRefreshFetch event to timefilter * replace fetchAndRender with reloadVis remove listener from editor * unsubscribe correct handler

nreese mentioned this pull request Jul 24, 2018

[6.x] add autoRefreshFetch event to timefilter (#20863) #21148

Merged

nreese mentioned this pull request Jul 24, 2018

auto refresh service #21150

Closed

timroes mentioned this pull request Jul 24, 2018

TypeScriptify visualize loader #21025

Merged

timroes pushed a commit that referenced this pull request Jul 24, 2018

add autoRefreshFetch event to timefilter (#20863) (#21148)

2170ab0

* add autoRefreshFetch event to timefilter * replace fetchAndRender with reloadVis remove listener from editor * unsubscribe correct handler

cjcenizal mentioned this pull request Aug 30, 2018

Timeout screen triggers with autorefresh and tabbing #22466

Closed

Conversation

nreese commented Jul 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

spalger Jul 16, 2018

Choose a reason for hiding this comment

Uh oh!

timroes Jul 17, 2018

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Jul 16, 2018

💚 Build Succeeded

Uh oh!

timroes Jul 17, 2018

Choose a reason for hiding this comment

Uh oh!

timroes Jul 17, 2018

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Jul 18, 2018

💔 Build Failed

Uh oh!

elasticmachine commented Jul 23, 2018

💔 Build Failed

Uh oh!

nreese commented Jul 24, 2018

Uh oh!

elasticmachine commented Jul 24, 2018

💔 Build Failed

Uh oh!

timroes commented Jul 24, 2018

Uh oh!

elasticmachine commented Jul 24, 2018

💔 Build Failed

Uh oh!

timroes commented Jul 24, 2018

Uh oh!

elasticmachine commented Jul 24, 2018

💔 Build Failed

Uh oh!

ppisljar left a comment

Choose a reason for hiding this comment

Uh oh!

timroes commented Jul 24, 2018

Uh oh!

elasticmachine commented Jul 24, 2018

💚 Build Succeeded

Uh oh!

ppisljar left a comment

Choose a reason for hiding this comment

Uh oh!

timroes left a comment

Choose a reason for hiding this comment

Uh oh!

nreese commented Jul 24, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nreese commented Jul 16, 2018 •

edited

Loading