Skip to content

Issue 1424 - Latest flask-compress (1.6.0) breaks Dash#1426

Merged
Marc-Andre-Rivet merged 9 commits intodevfrom
1424-flask-compress-brotli
Oct 6, 2020
Merged

Issue 1424 - Latest flask-compress (1.6.0) breaks Dash#1426
Marc-Andre-Rivet merged 9 commits intodevfrom
1424-flask-compress-brotli

Conversation

@Marc-Andre-Rivet
Copy link
Copy Markdown
Contributor

@Marc-Andre-Rivet Marc-Andre-Rivet commented Oct 6, 2020

Fixes #1424

Proposes to override the default COMPRESSION_ALGORITHM to ['gzip'] so as to avoid the costly, and currently non-configurable, Brotli compression.

Comment thread dash/dash.py Outdated
# flask-compress==1.6.0 changed default to ['br', 'gzip']
# and non-overridable default compression with Brotli is
# causing performance issues
self.server.config["COMPRESS_ALGORITHM"] = ["gzip"]
Copy link
Copy Markdown
Contributor Author

@Marc-Andre-Rivet Marc-Andre-Rivet Oct 6, 2020

Choose a reason for hiding this comment

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

Hesitant to override the values for servers passed as parameter https://github.com/plotly/dash/pull/1426/files#diff-228c9aefac729977642672aa1416bacaR277 as modifying a passed argument's settings would be unexpected, but maybe we don't really have a choice here.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Whenever we call Compress(self.server) we're effectively modifying the server's settings - and by default we do this even for user-provided servers. Perhaps we can check whether server.config["COMPRESS_ALGORITHM"] already has a value, and only set it if not?

Copy link
Copy Markdown
Contributor Author

@Marc-Andre-Rivet Marc-Andre-Rivet Oct 6, 2020

Choose a reason for hiding this comment

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

Turns out that running the code above against 1.5.0 fails and there is no __version__ or VERSION on flask_compress so determining the version needs to be done through pkg_resources. 01bfbcd

Copy link
Copy Markdown
Contributor Author

@Marc-Andre-Rivet Marc-Andre-Rivet Oct 6, 2020

Choose a reason for hiding this comment

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

Perhaps we can check whether server.config["COMPRESS_ALGORITHM"] already has a value, and only set it if not?

880df55

@Marc-Andre-Rivet Marc-Andre-Rivet marked this pull request as ready for review October 6, 2020 20:48
Comment thread requires-install.txt
@@ -1,5 +1,5 @@
Flask>=1.0.2
flask-compress==1.5.0
flask-compress
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

unfix flask-compress

Copy link
Copy Markdown
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

LGTM! 💃

@Marc-Andre-Rivet Marc-Andre-Rivet merged commit 63bb42f into dev Oct 6, 2020
@Marc-Andre-Rivet Marc-Andre-Rivet deleted the 1424-flask-compress-brotli branch October 6, 2020 21:37
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.

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

2 participants