chore: Bump flask libs #22355

EugeneTorap · 2022-12-07T08:54:21Z

SUMMARY

The PR bump the next flask libs:

flask to 2.1.3
flask-compress to 1.13
flask-login to 0.6.0
flask-talisman to 1.0.0
flask-wtf to 1.0.1
werkzeug to 2.1.2

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

requirements/base.txt

codecov · 2022-12-07T16:49:54Z

Codecov Report

Merging #22355 (a5fa09d) into master (6e4d6e5) will increase coverage by 0.00%.
The diff coverage is 96.00%.

❗ Current head a5fa09d differs from pull request most recent head 3f85e77. Consider uploading reports for the commit 3f85e77 to get more accurate results

@@           Coverage Diff           @@
##           master   #22355   +/-   ##
=======================================
  Coverage   67.00%   67.01%           
=======================================
  Files        1859     1859           
  Lines       71019    71037   +18     
  Branches     7766     7766           
=======================================
+ Hits        47587    47605   +18     
  Misses      21410    21410           
  Partials     2022     2022

Flag	Coverage Δ
hive	`52.46% <43.47%> (-0.01%)`	⬇️
mysql	`78.00% <95.65%> (+0.01%)`	⬆️
postgres	`78.07% <95.65%> (+0.01%)`	⬆️
presto	`52.36% <43.47%> (-0.01%)`	⬇️
python	`81.33% <95.65%> (+<0.01%)`	⬆️
sqlite	`76.45% <95.65%> (+0.01%)`	⬆️
unit	`51.46% <43.47%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...s/legacy-plugin-chart-country-map/src/countries.ts	`100.00% <ø> (ø)`
superset/views/core.py	`74.96% <75.00%> (-0.02%)`	⬇️
...c/components/Chart/DrillDetail/DrillDetailPane.tsx	`76.04% <100.00%> (ø)`
...mponents/DataTablesPane/components/SamplesPane.tsx	`97.67% <100.00%> (ø)`
superset/config.py	`91.66% <100.00%> (+0.12%)`	⬆️
superset/dashboards/api.py	`92.57% <100.00%> (ø)`
superset/views/filters.py	`100.00% <100.00%> (ø)`
superset/views/datasource/schemas.py	`97.22% <0.00%> (-2.78%)`	⬇️
superset/views/base.py	`76.97% <0.00%> (+0.68%)`	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

EugeneTorap · 2022-12-07T21:34:39Z

tests/integration_tests/core_tests.py

@@ -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"] == "/"


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.

EugeneTorap · 2022-12-07T21:37:20Z

tests/integration_tests/datasource_tests.py

    # get from cache
-    rv = test_client.post(uri)
+    rv = test_client.post(uri, json={})


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

dpgaspar · 2022-12-16T15:16:12Z

Seeing that GHSA-7wxw-4483-3m34 is a disputed CVE, and only occurs when running Flask with the provided dev server, something you should never do on production for several reasons.

regarding flask-caching I don't see on the CVE any submitted patches, GHSA-656c-6cxf-hvcv. This CVE is disputed also, since an attacker would have to be able to write to the cache itself, something that would cause severe issues all around not just by deserializing content with Pickle.

Can you please just bump Pillow and resolve the current PR conflicts? Thank you

EugeneTorap · 2022-12-21T09:58:32Z

@dpgaspar I've created a separate PR for Pillow - #22489
This PR will be for updating flask libs.

# Conflicts: # tests/integration_tests/thumbnails_tests.py

dpgaspar

looks good, final comments

dpgaspar · 2023-01-05T09:41:24Z

tests/integration_tests/conftest.py

+        app.config.get("SQLALCHEMY_EXAMPLES_URI")
+        or app.config["SQLALCHEMY_DATABASE_URI"]
+    )
+    backend = db_uri.split("://")[0]


this seems like an unrelated change, no?

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

dpgaspar · 2023-01-05T09:42:43Z

tests/integration_tests/datasource_tests.py

@@ -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


why did this changed to a 400 now?

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

EugeneTorap · 2023-01-06T09:29:28Z

@dpgaspar @villebro I have fixed that 400 'Bad Request' problem for datasource/samples [POST] API.
Can you take a look and I guess we can merge the PR.

dpgaspar · 2023-01-09T10:10:22Z

Great work! @EugeneTorap

cwegener · 2023-01-11T06:19:38Z

@EugeneTorap @dpgaspar Seeing that the compiled .txt lock files are manually edited in this change as well, is it correct to assume that pip-compile-multi is now deprecated and no longer used? Will it be replaced with an alternative any time soon?

EugeneTorap · 2023-01-11T09:14:08Z

Hi @cwegener! I want to prepare a PR with poetry instead of pip-compile-multi usage for dependency management.

villebro · 2023-01-11T09:44:36Z

@cwegener @EugeneTorap the lock files should preferably be updated via pip-compile-multi, and IMO it does work quite ok. Having said that, I'd love to see us move over to poetry, but I think it's a pretty big task and requires proper coordination to avoid unnecessary regressions or general back and forth.

cwegener · 2023-01-11T09:59:47Z

I find that poetry's dependency management is not quite reliable enough for large codebases.

Anyway. Sounds like there is a need for either discussion of the Python dependency management options, or a better checking of consistent pip-compile-multi usage.

villebro · 2023-01-11T10:02:28Z

I find that poetry's dependency management is not quite reliable enough for large codebases.

Anyway. Sounds like there is a need for either discussion of the Python dependency management options, or a better checking of consistent pip-compile-multi usage.

We should probably add a check to to our linting workflow ensure the compiled lock files are correct. Should be easy.

chore: Bump flask, cachelib and Pillow

15735c5

pull-request-size bot added the size/M label Dec 7, 2022

dpgaspar reviewed Dec 7, 2022

View reviewed changes

requirements/base.txt Show resolved Hide resolved

requirements/base.txt Outdated Show resolved Hide resolved

requirements/base.txt Outdated Show resolved Hide resolved

pull-request-size bot added size/L and removed size/M labels Dec 7, 2022

chore: Bump flask, cachelib and Pillow

7ee6bf5

pull-request-size bot added size/M and removed size/L labels Dec 7, 2022

Downgrade flask

b7cc110

pull-request-size bot added size/L size/M and removed size/M size/L labels Dec 7, 2022

EugeneTorap commented Dec 7, 2022

View reviewed changes

Upgrade libs

eb75516

pull-request-size bot added size/L and removed size/M labels Dec 8, 2022

Upgrade caching libs

b725517

EugeneTorap changed the title ~~chore: Bump flask, cachelib and Pillow~~ chore: Bump flask libs Dec 21, 2022

Merge branch 'master' into chore/bump-python-libs

6d891cf

# Conflicts: # tests/integration_tests/thumbnails_tests.py

pull-request-size bot added size/M and removed size/L labels Dec 24, 2022

Update only flask libs

03a1b26

EugeneTorap requested a review from dpgaspar December 26, 2022 13:51

EugeneTorap mentioned this pull request Dec 26, 2022

redirects change https requests to http locations #978

Closed

dpgaspar reviewed Jan 5, 2023

View reviewed changes

EugeneTorap added 2 commits January 5, 2023 14:54

Merge branch 'master' into chore/bump-python-libs

176c5d7

Fix empty payload for 'datasource/samples' API

3f85e77

dpgaspar approved these changes Jan 9, 2023

View reviewed changes

dpgaspar merged commit 30dab3a into apache:master Jan 9, 2023

EugeneTorap deleted the chore/bump-python-libs branch January 9, 2023 11:04

villebro mentioned this pull request Jan 11, 2023

fix(embed): fix server error due to breaking change on flask-login #22462

Merged

9 tasks

rusackas mentioned this pull request Jan 25, 2023

ImportError: cannot import name 'soft_unicode' from 'markupsafe' #19150

Closed

john-bodley mentioned this pull request Jul 11, 2023

chore: Bump/relax Flask libs #24652

Merged

9 tasks

mistercrunch added the 🚢 2.1.3 label Feb 18, 2024

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 and removed 🚢 2.1.3 labels Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Bump flask libs #22355

chore: Bump flask libs #22355

EugeneTorap commented Dec 7, 2022 •

edited

Loading

codecov bot commented Dec 7, 2022 •

edited

Loading

EugeneTorap Dec 7, 2022 •

edited

Loading

EugeneTorap Dec 7, 2022 •

edited

Loading

dpgaspar commented Dec 16, 2022 •

edited

Loading

EugeneTorap commented Dec 21, 2022

dpgaspar left a comment

dpgaspar Jan 5, 2023

EugeneTorap Jan 5, 2023 •

edited

Loading

dpgaspar Jan 5, 2023

EugeneTorap Jan 5, 2023

EugeneTorap commented Jan 6, 2023

dpgaspar commented Jan 9, 2023

cwegener commented Jan 11, 2023

EugeneTorap commented Jan 11, 2023

villebro commented Jan 11, 2023

cwegener commented Jan 11, 2023

villebro commented Jan 11, 2023

chore: Bump flask libs #22355

chore: Bump flask libs #22355

Conversation

EugeneTorap commented Dec 7, 2022 • edited Loading

SUMMARY

ADDITIONAL INFORMATION

codecov bot commented Dec 7, 2022 • edited Loading

Codecov Report

EugeneTorap Dec 7, 2022 • edited Loading

Choose a reason for hiding this comment

EugeneTorap Dec 7, 2022 • edited Loading

Choose a reason for hiding this comment

dpgaspar commented Dec 16, 2022 • edited Loading

EugeneTorap commented Dec 21, 2022

dpgaspar left a comment

Choose a reason for hiding this comment

dpgaspar Jan 5, 2023

Choose a reason for hiding this comment

EugeneTorap Jan 5, 2023 • edited Loading

Choose a reason for hiding this comment

dpgaspar Jan 5, 2023

Choose a reason for hiding this comment

EugeneTorap Jan 5, 2023

Choose a reason for hiding this comment

EugeneTorap commented Jan 6, 2023

dpgaspar commented Jan 9, 2023

cwegener commented Jan 11, 2023

EugeneTorap commented Jan 11, 2023

villebro commented Jan 11, 2023

cwegener commented Jan 11, 2023

villebro commented Jan 11, 2023

EugeneTorap commented Dec 7, 2022 •

edited

Loading

codecov bot commented Dec 7, 2022 •

edited

Loading

EugeneTorap Dec 7, 2022 •

edited

Loading

EugeneTorap Dec 7, 2022 •

edited

Loading

dpgaspar commented Dec 16, 2022 •

edited

Loading

EugeneTorap Jan 5, 2023 •

edited

Loading