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

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

Merged
merged 9 commits into from
Oct 6, 2020
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions dash/dash.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,11 @@ def __init__(
elif isinstance(server, bool):
name = name if name else "__main__"
self.server = flask.Flask(name) if server else None
if self.server is not None:
# 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
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
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
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
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

else:
raise ValueError("server must be a Flask app or a boolean")

Expand Down