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

[BUG] Latest flask-compress (1.6.0) breaks Dash #1424

Closed
Marc-Andre-Rivet opened this issue Oct 6, 2020 · 1 comment · Fixed by #1426
Closed

[BUG] Latest flask-compress (1.6.0) breaks Dash #1424

Marc-Andre-Rivet opened this issue Oct 6, 2020 · 1 comment · Fixed by #1426
Milestone

Comments

@Marc-Andre-Rivet
Copy link
Contributor

Marc-Andre-Rivet commented Oct 6, 2020

flask-compress released version 1.6.0 on 2020-10-05 and updating to this version breaks component tests (confirmed in the table) in CI. Since flask-compress is a dash requirement and not version locked, this may affect negatively Dash installs in the wild.

  • Determine the nature of the break
  • Fix it..
@Marc-Andre-Rivet Marc-Andre-Rivet added this to the OSS milestone Oct 6, 2020
@Marc-Andre-Rivet
Copy link
Contributor Author

Marc-Andre-Rivet commented Oct 6, 2020

@alexcjohnson Got a clean repro of the issue by switching back and forth between flask-compress 1.5.0 and 1.6.0

The underlying issue seems to be that 1.6.0 changed the default compression algorithm from['gzip'] to ['br', 'gzip']. In doing so they also do not provide a way to change the default compression level of Brotli which is set to the maximum value of 11.

11 is a very aggressive compression value for Brotli with significant CPU cost - you would want to use it on static resources and do it once / cache the result instead of doing it on the fly like we do.

To give a sense of that cost, callbacks with marginal content (~150 bytes) that would normally return in 10-15ms on the localhost now take 150-200ms to return.

@rpkyle is opening a PR on flask-compress to make the default more sensible and to suggest making the compression level for br, like gzip, customizable.

All of this should have caused performance issues but should not have broken the table tests. The second underlying issue here is that the table tests are brittle when it comes to server slowdowns. For example, test_tbcp001_copy_paste_callback is written like this:

    assert target.cell(1, 0).get_text() == "0"
    assert target.cell(1, 1).get_text() == "MODIFIED"

and would be more resilient written like so:

    wait.until(lambda: target.cell(1, 0).get_text() == "0", 3)
    wait.until(lambda: target.cell(1, 1).get_text() == "MODIFIED", 3)

Sadly we would also have missed out on the huge performance cost of this update if the tests had been written correctly.

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

Successfully merging a pull request may close this issue.

1 participant