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.

Sign up for GitHub

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

Jump to bottom

chore: Bump flask libs #22355

Merged
merged 9 commits into from
Jan 9, 2023
Merged

chore: Bump flask libs #22355

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
15735c5
chore: Bump flask, cachelib and Pillow
EugeneTorap Dec 7, 2022
7ee6bf5
chore: Bump flask, cachelib and Pillow
EugeneTorap Dec 7, 2022
b7cc110
Downgrade flask
EugeneTorap Dec 7, 2022
eb75516
Upgrade libs
EugeneTorap Dec 7, 2022
b725517
Upgrade caching libs
EugeneTorap Dec 8, 2022
6d891cf
Merge branch 'master' into chore/bump-python-libs
EugeneTorap Dec 24, 2022
03a1b26
Update only flask libs
EugeneTorap Dec 24, 2022
176c5d7
Merge branch 'master' into chore/bump-python-libs
EugeneTorap Jan 5, 2023
3f85e77
Fix empty payload for 'datasource/samples' API
EugeneTorap Jan 5, 2023
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
16 changes: 8 additions & 8 deletions requirements/base.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ dnspython==2.1.0
# via email-validator
email-validator==1.1.3
# via flask-appbuilder
flask==2.0.3
flask==2.1.3
# via
# apache-superset
# flask-appbuilder
Expand All @@ -86,21 +86,21 @@ flask-babel==1.0.0
# via flask-appbuilder
flask-caching==1.10.1
# via apache-superset
flask-compress==1.10.1
flask-compress==1.13
# via apache-superset
flask-jwt-extended==4.3.1
# via flask-appbuilder
flask-login==0.4.1
flask-login==0.6.0
# via flask-appbuilder
flask-migrate==3.1.0
# via apache-superset
flask-sqlalchemy==2.5.1
# via
# flask-appbuilder
# flask-migrate
flask-talisman==0.8.1
flask-talisman==1.0.0
# via apache-superset
flask-wtf==0.14.3
flask-wtf==1.0.1
EugeneTorap marked this conversation as resolved.
Show resolved Hide resolved
# via
# apache-superset
# flask-appbuilder
Expand Down Expand Up @@ -144,10 +144,11 @@ mako==1.1.4
# via alembic
markdown==3.3.4
# via apache-superset
markupsafe==2.0.1
markupsafe==2.1.1
# via
# jinja2
# mako
# werkzeug
# wtforms
marshmallow==3.13.0
# via
Expand Down Expand Up @@ -236,7 +237,6 @@ six==1.16.0
# via
# bleach
# click-repl
# flask-talisman
# isodate
# jsonschema
# paramiko
Expand Down Expand Up @@ -278,7 +278,7 @@ wcwidth==0.2.5
# via prompt-toolkit
webencodings==0.5.1
# via bleach
werkzeug==2.0.3
werkzeug==2.1.2
# via
# flask
# flask-jwt-extended
Expand Down
13 changes: 7 additions & 6 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,14 @@ def get_git_sha() -> str:
"cron-descriptor",
"cryptography>=3.3.2",
"deprecation>=2.1.0, <2.2.0",
"flask>=2.0.0, <3.0.0",
"flask>=2.1.3, <2.2",
"flask-appbuilder>=4.1.6, <5.0.0",
"flask-caching>=1.10.0",
"flask-compress",
"flask-talisman",
"flask-migrate",
"flask-wtf",
"flask-caching>=1.10.1, <1.11",
"flask-compress>=1.13, <2.0",
"flask-talisman>=1.0.0, <2.0",
"flask-login==0.6.0",
"flask-migrate>=3.1.0, <4.0",
"flask-wtf>=1.0.1, <1.1",
"func_timeout",
"geopy",
"graphlib-backport",
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ export default function DrillDetailPane({
useEffect(() => {
if (!responseError && !isLoading && !resultsPages.has(pageIndex)) {
setIsLoading(true);
const jsonPayload = getDrillPayload(formData, filters);
const jsonPayload = getDrillPayload(formData, filters) ?? {};
const cachePageLimit = Math.ceil(SAMPLES_ROW_LIMIT / PAGE_SIZE);
getDatasourceSamples(
datasourceType,
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/explore/components/DataTablesPane/components/SamplesPane.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export const SamplesPane = ({

if (isRequest && !cache.has(datasource)) {
setIsLoading(true);
getDatasourceSamples(datasource.type, datasource.id, queryForce)
getDatasourceSamples(datasource.type, datasource.id, queryForce, {})
.then(response => {
setData(ensureIsArray(response.data));
setColnames(ensureIsArray(response.colnames));
Expand Down
7 changes: 6 additions & 1 deletion superset/views/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -773,7 +773,12 @@ def get_redirect_url() -> str:
query["form_data_key"] = [form_data_key]
url = url._replace(query=parse.urlencode(query, True))
redirect_url = parse.urlunparse(url)
return redirect_url

# Return a relative URL
url = parse.urlparse(redirect_url)
if url.query:
return f"{url.path}?{url.query}"
return url.path

@has_access
@event_logger.log_this
Expand Down
6 changes: 5 additions & 1 deletion tests/integration_tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,11 @@ def remove(self) -> None:


def setup_presto_if_needed():
backend = app.config["SQLALCHEMY_EXAMPLES_URI"].split("://")[0]
db_uri = (
app.config.get("SQLALCHEMY_EXAMPLES_URI")
or app.config["SQLALCHEMY_DATABASE_URI"]
)
backend = db_uri.split("://")[0]
Copy link
Member

Choose a reason for hiding this comment

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

this seems like an unrelated change, no?

Copy link
Contributor Author

@EugeneTorap EugeneTorap Jan 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It's the related change. When you want to run the local tests with SQLALCHEMY_EXAMPLES_URI = None you will have an exception here and I fix the issue.

URI to database storing the example data, points to
SQLALCHEMY_DATABASE_URI by default if set to None

database = get_example_database()
extra = database.get_extra()

Expand Down
6 changes: 4 additions & 2 deletions tests/integration_tests/core_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -626,7 +626,7 @@ def test_redirect_invalid(self):

self.login(username="admin")
response = self.client.get(f"/r/{model_url.id}")
assert response.headers["Location"] == "http://localhost/"
assert response.headers["Location"] == "/"
Copy link
Contributor Author

@EugeneTorap EugeneTorap Dec 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Werkzeug 2.1.0 changelog:

Response.autocorrect_location_header is disabled by default. The Location header URL will remain relative, and exclude the scheme and domain, by default. pallets/werkzeug#2352

When Response.autocorrect_location_header was added in 2011 pallets/werkzeug@0ec643b, it was documented as "correct the location header to be RFC conformant". It was referring to RFC 2616. That was superseded by RFC 7231 in 2014, which allows relative URLs. MDN lists all browsers as compliant with this. Switch autocorrect_location_header to be disabled by default.

db.session.delete(model_url)
db.session.commit()

Expand Down Expand Up @@ -1671,7 +1671,9 @@ def test_explore_redirect(self, mock_command: mock.Mock):
rv = self.client.get(
f"/superset/explore/?form_data={quote(json.dumps(form_data))}"
)
self.assertRedirects(rv, f"/explore/?form_data_key={random_key}")
self.assertEqual(
rv.headers["Location"], f"/explore/?form_data_key={random_key}"
)


if __name__ == "__main__":
Expand Down
2 changes: 1 addition & 1 deletion tests/integration_tests/dashboard_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ def test_new_dashboard(self):
dash_count_after = db.session.query(func.count(Dashboard.id)).first()[0]
self.assertEqual(dash_count_before + 1, dash_count_after)
group = re.match(
r"http:\/\/localhost\/superset\/dashboard\/([0-9]*)\/\?edit=true",
r"\/superset\/dashboard\/([0-9]*)\/\?edit=true",
response.headers["Location"],
)
assert group is not None
Expand Down
28 changes: 14 additions & 14 deletions tests/integration_tests/datasource_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,9 +466,9 @@ def test_get_samples(test_client, login_as_admin, virtual_dataset):
f"/datasource/samples?datasource_id={virtual_dataset.id}&datasource_type=table"
)
# feeds data
test_client.post(uri)
test_client.post(uri, json={})
# get from cache
rv = test_client.post(uri)
rv = test_client.post(uri, json={})
Copy link
Contributor Author

@EugeneTorap EugeneTorap Dec 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Werkzeug 2.1.0 changelog:

Request.get_json() will raise a 400 BadRequest error if the Content-Type header is not application/json. This makes a very common source of confusion more visible. pallets/werkzeug#2339

If there's no json param or json=None then 400 http status will be thrown

assert rv.status_code == 200
assert len(rv.json["result"]["data"]) == 10
assert QueryCacheManager.has(
Expand All @@ -480,9 +480,9 @@ def test_get_samples(test_client, login_as_admin, virtual_dataset):
# 2. should read through cache data
uri2 = f"/datasource/samples?datasource_id={virtual_dataset.id}&datasource_type=table&force=true"
# feeds data
test_client.post(uri2)
test_client.post(uri2, json={})
# force query
rv2 = test_client.post(uri2)
rv2 = test_client.post(uri2, json={})
assert rv2.status_code == 200
assert len(rv2.json["result"]["data"]) == 10
assert QueryCacheManager.has(
Expand Down Expand Up @@ -518,7 +518,7 @@ def test_get_samples_with_incorrect_cc(test_client, login_as_admin, virtual_data
uri = (
f"/datasource/samples?datasource_id={virtual_dataset.id}&datasource_type=table"
)
rv = test_client.post(uri)
rv = test_client.post(uri, json={})
assert rv.status_code == 422

assert "error" in rv.json
Expand All @@ -530,7 +530,7 @@ def test_get_samples_on_physical_dataset(test_client, login_as_admin, physical_d
uri = (
f"/datasource/samples?datasource_id={physical_dataset.id}&datasource_type=table"
)
rv = test_client.post(uri)
rv = test_client.post(uri, json={})
assert rv.status_code == 200
assert QueryCacheManager.has(
rv.json["result"]["cache_key"], region=CacheRegion.DATA
Expand All @@ -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 == 200
assert rv.status_code == 400
Copy link
Member

Choose a reason for hiding this comment

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

why did this changed to a 400 now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Werkzeug 2.1.0 changelog:

Request.get_json() will raise a 400 BadRequest error if the Content-Type header is not application/json. This makes a very common source of confusion more visible. pallets/werkzeug#2339

If there's no json param or json=None then 400 http status will be thrown


rv = test_client.post(uri, json={})
assert rv.status_code == 200
Expand Down Expand Up @@ -644,7 +644,7 @@ def test_get_samples_pagination(test_client, login_as_admin, virtual_dataset):
uri = (
f"/datasource/samples?datasource_id={virtual_dataset.id}&datasource_type=table"
)
rv = test_client.post(uri)
rv = test_client.post(uri, json={})
assert rv.json["result"]["page"] == 1
assert rv.json["result"]["per_page"] == app.config["SAMPLES_ROW_LIMIT"]
assert rv.json["result"]["total_count"] == 10
Expand All @@ -653,36 +653,36 @@ def test_get_samples_pagination(test_client, login_as_admin, virtual_dataset):
per_pages = (app.config["SAMPLES_ROW_LIMIT"] + 1, 0, "xx")
for per_page in per_pages:
uri = f"/datasource/samples?datasource_id={virtual_dataset.id}&datasource_type=table&per_page={per_page}"
rv = test_client.post(uri)
rv = test_client.post(uri, json={})
assert rv.status_code == 400

# 3. incorrect page or datasource_type
uri = f"/datasource/samples?datasource_id={virtual_dataset.id}&datasource_type=table&page=xx"
rv = test_client.post(uri)
rv = test_client.post(uri, json={})
assert rv.status_code == 400

uri = f"/datasource/samples?datasource_id={virtual_dataset.id}&datasource_type=xx"
rv = test_client.post(uri)
rv = test_client.post(uri, json={})
assert rv.status_code == 400

# 4. turning pages
uri = f"/datasource/samples?datasource_id={virtual_dataset.id}&datasource_type=table&per_page=2&page=1"
rv = test_client.post(uri)
rv = test_client.post(uri, json={})
assert rv.json["result"]["page"] == 1
assert rv.json["result"]["per_page"] == 2
assert rv.json["result"]["total_count"] == 10
assert [row["col1"] for row in rv.json["result"]["data"]] == [0, 1]

uri = f"/datasource/samples?datasource_id={virtual_dataset.id}&datasource_type=table&per_page=2&page=2"
rv = test_client.post(uri)
rv = test_client.post(uri, json={})
assert rv.json["result"]["page"] == 2
assert rv.json["result"]["per_page"] == 2
assert rv.json["result"]["total_count"] == 10
assert [row["col1"] for row in rv.json["result"]["data"]] == [2, 3]

# 5. Exceeding the maximum pages
uri = f"/datasource/samples?datasource_id={virtual_dataset.id}&datasource_type=table&per_page=2&page=6"
rv = test_client.post(uri)
rv = test_client.post(uri, json={})
assert rv.json["result"]["page"] == 6
assert rv.json["result"]["per_page"] == 2
assert rv.json["result"]["total_count"] == 10
Expand Down
4 changes: 2 additions & 2 deletions tests/integration_tests/thumbnails_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ def test_get_cached_chart_wrong_digest(self):
id_, thumbnail_url = self._get_id_and_thumbnail_url(CHART_URL)
rv = self.client.get(f"api/v1/chart/{id_}/thumbnail/1234/")
self.assertEqual(rv.status_code, 302)
self.assertRedirects(rv, thumbnail_url)
self.assertEqual(rv.headers["Location"], thumbnail_url)

@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
@with_feature_flags(THUMBNAILS=True)
Expand Down Expand Up @@ -413,4 +413,4 @@ def test_get_cached_dashboard_wrong_digest(self):
id_, thumbnail_url = self._get_id_and_thumbnail_url(DASHBOARD_URL)
rv = self.client.get(f"api/v1/dashboard/{id_}/thumbnail/1234/")
self.assertEqual(rv.status_code, 302)
self.assertRedirects(rv, thumbnail_url)
self.assertEqual(rv.headers["Location"], thumbnail_url)