Skip to content

[5.6] Backport. limit wait time for baselayer (#14047) - #14305

Closed
thomasneirynck wants to merge 2 commits into
elastic:5.6from
thomasneirynck:5.6_5e713df81e49d87c1e6f82507c28a8416681c900
Closed

thomasneirynck wants to merge 2 commits into
elastic:5.6from
thomasneirynck:5.6_5e713df81e49d87c1e6f82507c28a8416681c900

Conversation

@thomasneirynck

@thomasneirynck thomasneirynck commented Oct 4, 2017 •

Copy link
Copy Markdown
Contributor

This is a manual backport of #14047.

Due to changes from v5 to v6, the implementation is slightly different.

@thomasneirynck thomasneirynck added Feature:Visualizations Generic visualization features (in case no more specific feature label is available) backport This PR is a backport of another PR review v5.6.3 labels Oct 4, 2017
@thomasneirynck
thomasneirynck requested a review from kobelb October 4, 2017 16:28
@thomasneirynck thomasneirynck changed the title limit wait time for baselayer (#14047) [5.6] Backport. limit wait time for baselayer (#14047) Oct 4, 2017
@epixa epixa added v5.6.4 and removed v5.6.3 labels Oct 11, 2017
this._doRenderCompleteWhenBaseLayerIsLoaded(resolve, Date.now() + msAllowedForBaseLayerToLoad);
}


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason we have all this whitespace here?

if (this._paramsDirty || this._dataDirty || this._baseLayerDirty) {
return;
if (Date.now() <= endTime) {
setTimeout(() => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like we could be using (endTime - Date.now()) to only do one setTimeout that fires the renderComplete if we haven't yet. I'm not understanding why we continually call _doRenderCompleteWhenBaseLayerIsLoaded every 10 ms recursively...

@thomasneirynck

Copy link
Copy Markdown
Contributor Author

just to give an update. this isn't working. needs to be fixed before we can merge this into 5.6. cc @bhavyarm. I'll try and do this tomorrow, thx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport This PR is a backport of another PR Feature:Visualizations Generic visualization features (in case no more specific feature label is available) review stalled v5.6.6

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants