Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create EventSource at the right moment #182

Closed
wants to merge 1 commit into from

Conversation

toy
Copy link
Contributor

@toy toy commented Mar 4, 2021

Closes #180

On small dashboard setTimeout with timeout 0 was enough to create
EventSource after all widgets were initialised, but on big dashboards
the ready event of layout (View instance for document) is the only event
that is happening after all widgets get initialised and registered in
the list.

On small dashboard setTimeout with timeout 0 was enough to create
EventSource after all widgets were initialised, but on big dashboards
the ready event of layout (View instance for document) is the only event
that is happening after all widgets get initialised and registered in
the list.
Copy link
Member

@kinow kinow left a comment

Choose a reason for hiding this comment

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

@toy have you tested with 160 widgets and with long names too? I think for #181 the length of the generated URL wouldn't be fixed with only this change.

@kinow kinow changed the title Create EventSource at the right moment, resolves #181 Create EventSource at the right moment Mar 6, 2021
@kinow
Copy link
Member

kinow commented Mar 6, 2021

Confirmed it fixes #180 , not sure about #181 (URL length), so leaving that one open for now. Preparing a release with this PR, thanks again @toy!

@kinow kinow added this to the 1.3.3 milestone Mar 6, 2021
@kinow kinow closed this in 05a1314 Mar 6, 2021
@kinow
Copy link
Member

kinow commented Mar 6, 2021

Added changelog, pushed to master with the commit merged 👍

@kinow kinow self-assigned this Mar 6, 2021
@toy
Copy link
Contributor Author

toy commented Mar 6, 2021

@toy have you tested with 160 widgets and with long names too? I think for #181 the length of the generated URL wouldn't be fixed with only this change.

Locally (in docker image), so using thin, it works with 200 widgets each having id of 44 characters (longest in the issue was 42) resulting in a query string of 9401. Which confirms that all depends on the server and proxies on the way. But if one of them doesn't handle url or query string that long, the error should be very explicit with status 414 and no widget getting any data.
Apparently it is not possible to get the status on error event for EventSource, so no way to automatically switch to not sending ids.

@toy toy deleted the right-time-to-be-ready branch March 6, 2021 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Smashing not displaying data
2 participants