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

chore: bump werkzeug and Flask #23965

Merged
merged 27 commits into from May 12, 2023
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 6 additions & 5 deletions requirements/base.txt
Expand Up @@ -35,7 +35,7 @@ cffi==1.15.1
# via
# cryptography
# pynacl
click==8.0.4
click==8.1.3
# via
# apache-superset
# celery
Expand Down Expand Up @@ -75,7 +75,7 @@ dnspython==2.1.0
# via email-validator
email-validator==1.1.3
# via flask-appbuilder
flask==2.1.3
flask==2.2.5
# via
# apache-superset
# flask-appbuilder
Expand Down Expand Up @@ -144,11 +144,11 @@ importlib-resources==5.12.0
# via limits
isodate==0.6.0
# via apache-superset
itsdangerous==2.1.1
itsdangerous==2.1.2
# via
# flask
# flask-wtf
jinja2==3.0.3
jinja2==3.1.2
# via
# flask
# flask-babel
Expand All @@ -170,6 +170,7 @@ markupsafe==2.1.1
# via
# jinja2
# mako
# werkzeug
# wtforms
marshmallow==3.13.0
# via
Expand Down Expand Up @@ -306,7 +307,7 @@ vine==5.0.0
# kombu
wcwidth==0.2.5
# via prompt-toolkit
werkzeug==2.1.2
werkzeug==2.3.3
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest you to pin werkzeug in setup.py because we use this lib in superset project directly.

Copy link
Member Author

Choose a reason for hiding this comment

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

done, makes sense

# via
# flask
# flask-jwt-extended
Expand Down
14 changes: 7 additions & 7 deletions requirements/integration.txt
Expand Up @@ -11,9 +11,7 @@ cachetools==5.3.0
# via tox
cfgv==3.3.1
# via pre-commit
chardet==5.1.0
# via tox
click==8.0.4
click==8.1.3
# via
# pip-compile-multi
# pip-tools
Expand Down Expand Up @@ -46,17 +44,19 @@ pluggy==1.0.0
# via tox
pre-commit==3.3.1
# via -r requirements/integration.in
py==1.10.0
pyproject-api==1.5.1
# via tox
pyproject-hooks==1.0.0
# via build
pyyaml==5.4.1
# via pre-commit
six==1.16.0
dpgaspar marked this conversation as resolved.
Show resolved Hide resolved
# via tox
toml==0.10.2
# via tox
tomli==2.0.1
# via
# build
# pyproject-api
# tox
# via build
toposort==1.10
# via pip-compile-multi
tox==4.5.1
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Expand Up @@ -82,7 +82,7 @@ def get_git_sha() -> str:
"cron-descriptor",
"cryptography>=39.0.1, <40",
"deprecation>=2.1.0, <2.2.0",
"flask>=2.1.3, <2.2",
"flask>=2.2.5, <3.0.0",
"flask-appbuilder>=4.3.0, <5.0.0",
"flask-caching>=1.10.1, <1.11",
"flask-compress>=1.13, <2.0",
Expand Down
2 changes: 1 addition & 1 deletion superset/charts/api.py
Expand Up @@ -805,7 +805,7 @@ def export(self, **kwargs: Any) -> Response:
buf,
mimetype="application/zip",
as_attachment=True,
attachment_filename=filename,
download_name=filename,
)
if token:
response.set_cookie(token, "done", max_age=600)
Expand Down
2 changes: 1 addition & 1 deletion superset/dashboards/api.py
Expand Up @@ -819,7 +819,7 @@ def export(self, **kwargs: Any) -> Response:
buf,
mimetype="application/zip",
as_attachment=True,
attachment_filename=filename,
download_name=filename,
)
if token:
response.set_cookie(token, "done", max_age=600)
Expand Down
2 changes: 1 addition & 1 deletion superset/databases/api.py
Expand Up @@ -1058,7 +1058,7 @@ def export(self, **kwargs: Any) -> Response:
buf,
mimetype="application/zip",
as_attachment=True,
attachment_filename=filename,
download_name=filename,
)
if token:
response.set_cookie(token, "done", max_age=600)
Expand Down
2 changes: 1 addition & 1 deletion superset/datasets/api.py
Expand Up @@ -524,7 +524,7 @@ def export(self, **kwargs: Any) -> Response:
buf,
mimetype="application/zip",
as_attachment=True,
attachment_filename=filename,
download_name=filename,
)
if token:
response.set_cookie(token, "done", max_age=600)
Expand Down
2 changes: 1 addition & 1 deletion superset/importexport/api.py
Expand Up @@ -87,7 +87,7 @@ def export(self) -> Response:
buf,
mimetype="application/zip",
as_attachment=True,
attachment_filename=filename,
download_name=filename,
)
return response

Expand Down
2 changes: 1 addition & 1 deletion superset/queries/saved_queries/api.py
Expand Up @@ -284,7 +284,7 @@ def export(self, **kwargs: Any) -> Response:
buf,
mimetype="application/zip",
as_attachment=True,
attachment_filename=filename,
download_name=filename,
)
if token:
response.set_cookie(token, "done", max_age=600)
Expand Down
6 changes: 3 additions & 3 deletions superset/views/base.py
Expand Up @@ -490,7 +490,7 @@ def show_http_exception(ex: HTTPException) -> FlaskResponse:
and ex.code in {404, 500}
):
path = resource_filename("superset", f"static/assets/{ex.code}.html")
return send_file(path, cache_timeout=0), ex.code
return send_file(path, max_age=0), ex.code

return json_errors_response(
errors=[
Expand All @@ -512,7 +512,7 @@ def show_command_errors(ex: CommandException) -> FlaskResponse:
logger.warning("CommandException", exc_info=True)
if "text/html" in request.accept_mimetypes and not config["DEBUG"]:
path = resource_filename("superset", "static/assets/500.html")
return send_file(path, cache_timeout=0), 500
return send_file(path, max_age=0), 500

extra = ex.normalized_messages() if isinstance(ex, CommandInvalidError) else {}
return json_errors_response(
Expand All @@ -534,7 +534,7 @@ def show_unexpected_exception(ex: Exception) -> FlaskResponse:
logger.exception(ex)
if "text/html" in request.accept_mimetypes and not config["DEBUG"]:
path = resource_filename("superset", "static/assets/500.html")
return send_file(path, cache_timeout=0), 500
return send_file(path, max_age=0), 500

return json_errors_response(
errors=[
Expand Down
4 changes: 4 additions & 0 deletions tests/integration_tests/async_events/api_tests.py
Expand Up @@ -33,6 +33,7 @@ def fetch_events(self, last_id: Optional[str] = None):

@mock.patch("uuid.uuid4", return_value=UUID)
def test_events(self, mock_uuid4):
app._got_first_request = False
Copy link
Member Author

@dpgaspar dpgaspar May 11, 2023

Choose a reason for hiding this comment

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

a bit of an hack, async_query_manager.init_app(app) registers a Flask app.before_request. New Flask will only allow registering before request hooks if no request was yet processed by the app.

async_query_manager.init_app(app)
self.login(username="admin")
with mock.patch.object(async_query_manager._redis, "xrange") as mock_xrange:
Expand All @@ -46,6 +47,7 @@ def test_events(self, mock_uuid4):

@mock.patch("uuid.uuid4", return_value=UUID)
def test_events_last_id(self, mock_uuid4):
app._got_first_request = False
async_query_manager.init_app(app)
self.login(username="admin")
with mock.patch.object(async_query_manager._redis, "xrange") as mock_xrange:
Expand All @@ -59,6 +61,7 @@ def test_events_last_id(self, mock_uuid4):

@mock.patch("uuid.uuid4", return_value=UUID)
def test_events_results(self, mock_uuid4):
app._got_first_request = False
async_query_manager.init_app(app)
self.login(username="admin")
with mock.patch.object(async_query_manager._redis, "xrange") as mock_xrange:
Expand Down Expand Up @@ -107,6 +110,7 @@ def test_events_results(self, mock_uuid4):
self.assertEqual(response, expected)

def test_events_no_login(self):
app._got_first_request = False
async_query_manager.init_app(app)
rv = self.fetch_events()
assert rv.status_code == 401
Expand Down
18 changes: 11 additions & 7 deletions tests/integration_tests/charts/data/api_tests.py
Expand Up @@ -28,10 +28,7 @@
from flask import Response
from tests.integration_tests.conftest import with_feature_flags
from superset.models.sql_lab import Query
from tests.integration_tests.base_tests import (
SupersetTestCase,
test_client,
)
from tests.integration_tests.base_tests import SupersetTestCase, test_client
from tests.integration_tests.annotation_layers.fixtures import create_annotation_layers
from tests.integration_tests.fixtures.birth_names_dashboard import (
load_birth_names_dashboard_with_slices,
Expand Down Expand Up @@ -626,6 +623,7 @@ def test_when_where_parameter_is_template_and_query_result_type__query_is_templa
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_chart_data_async(self):
self.logout()
app._got_first_request = False
async_query_manager.init_app(app)
self.login("admin")
rv = self.post_assert_metric(CHART_DATA_URI, self.query_context_payload, "data")
Expand All @@ -643,6 +641,7 @@ def test_chart_data_async_cached_sync_response(self):
Chart data API: Test chart data query returns results synchronously
when results are already cached.
"""
app._got_first_request = False
async_query_manager.init_app(app)

class QueryContext:
Expand Down Expand Up @@ -672,6 +671,7 @@ def test_chart_data_async_results_type(self):
"""
Chart data API: Test chart data query non-JSON format (async)
"""
app._got_first_request = False
async_query_manager.init_app(app)
self.query_context_payload["result_type"] = "results"
rv = self.post_assert_metric(CHART_DATA_URI, self.query_context_payload, "data")
Expand All @@ -683,6 +683,7 @@ def test_chart_data_async_invalid_token(self):
"""
Chart data API: Test chart data query (async)
"""
app._got_first_request = False
async_query_manager.init_app(app)
test_client.set_cookie(
"localhost", app.config["GLOBAL_ASYNC_QUERIES_JWT_COOKIE_NAME"], "foo"
Expand Down Expand Up @@ -998,6 +999,7 @@ def test_chart_data_cache(self, cache_loader):
"""
Chart data cache API: Test chart data async cache request
"""
app._got_first_request = False
async_query_manager.init_app(app)
cache_loader.load.return_value = self.query_context_payload
orig_run = ChartDataCommand.run
Expand All @@ -1024,6 +1026,7 @@ def test_chart_data_cache_run_failed(self, cache_loader):
"""
Chart data cache API: Test chart data async cache request with run failure
"""
app._got_first_request = False
async_query_manager.init_app(app)
cache_loader.load.return_value = self.query_context_payload
rv = self.get_assert_metric(
Expand All @@ -1041,8 +1044,9 @@ def test_chart_data_cache_no_login(self, cache_loader):
"""
Chart data cache API: Test chart data async cache request (no login)
"""
self.logout()
app._got_first_request = False
async_query_manager.init_app(app)
self.logout()
cache_loader.load.return_value = self.query_context_payload
orig_run = ChartDataCommand.run

Expand All @@ -1063,6 +1067,7 @@ def test_chart_data_cache_key_error(self):
"""
Chart data cache API: Test chart data async cache request with invalid cache key
"""
app._got_first_request = False
async_query_manager.init_app(app)
rv = self.get_assert_metric(
f"{CHART_DATA_URI}/test-cache-key", "data_from_cache"
Expand Down Expand Up @@ -1188,10 +1193,10 @@ def test_data_cache_default_timeout(


def test_chart_cache_timeout(
load_energy_table_with_slice: List[Slice],
Copy link
Member Author

Choose a reason for hiding this comment

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

This is an interesting case that surfaced on this new Flask version. Flask now is stricter, so when popping a new context from the app stack it checks if the context is the one we are expecting. load_energy_table_with_slice creates a new app_context so it needs to be first

test_client,
login_as_admin,
physical_query_context,
load_energy_table_with_slice: List[Slice],
):
# should override datasource cache timeout

Expand All @@ -1210,7 +1215,6 @@ def test_chart_cache_timeout(
db.session.commit()

physical_query_context["form_data"] = {"slice_id": slice_with_cache_timeout.id}

rv = test_client.post(CHART_DATA_URI, json=physical_query_context)
assert rv.json["result"][0]["cache_timeout"] == 20

Expand Down
2 changes: 2 additions & 0 deletions tests/integration_tests/core_tests.py
Expand Up @@ -1088,6 +1088,7 @@ def test_explore_json_async(self):
"groupby": ["gender"],
"row_limit": 100,
}
app._got_first_request = False
async_query_manager.init_app(app)
self.login(username="admin")
rv = self.client.post(
Expand Down Expand Up @@ -1119,6 +1120,7 @@ def test_explore_json_async_results_format(self):
"groupby": ["gender"],
"row_limit": 100,
}
app._got_first_request = False
async_query_manager.init_app(app)
self.login(username="admin")
rv = self.client.post(
Expand Down
6 changes: 3 additions & 3 deletions tests/integration_tests/dashboards/permalink/api_tests.py
Expand Up @@ -66,7 +66,7 @@ def permalink_salt() -> Iterator[str]:


def test_post(
test_client, login_as_admin, dashboard_id: int, permalink_salt: str
dashboard_id: int, permalink_salt: str, test_client, login_as_admin
) -> None:
resp = test_client.post(f"api/v1/dashboard/{dashboard_id}/permalink", json=STATE)
assert resp.status_code == 201
Expand All @@ -93,14 +93,14 @@ def test_post_access_denied(test_client, login_as, dashboard_id: int):
assert resp.status_code == 404


def test_post_invalid_schema(test_client, login_as_admin, dashboard_id: int):
def test_post_invalid_schema(dashboard_id: int, test_client, login_as_admin):
resp = test_client.post(
f"api/v1/dashboard/{dashboard_id}/permalink", json={"foo": "bar"}
)
assert resp.status_code == 400


def test_get(test_client, login_as_admin, dashboard_id: int, permalink_salt: str):
def test_get(dashboard_id: int, permalink_salt: str, test_client, login_as_admin):
key = test_client.post(
f"api/v1/dashboard/{dashboard_id}/permalink", json=STATE
).json["key"]
Expand Down
2 changes: 1 addition & 1 deletion tests/integration_tests/datasource_tests.py
Expand Up @@ -543,7 +543,7 @@ def test_get_samples_with_filters(test_client, login_as_admin, virtual_dataset):
f"/datasource/samples?datasource_id={virtual_dataset.id}&datasource_type=table"
)
rv = test_client.post(uri, json=None)
assert rv.status_code == 400
assert rv.status_code == 415

rv = test_client.post(uri, json={})
assert rv.status_code == 200
Expand Down