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

Loosen flask version requirement to >= 2 #479

Merged
merged 13 commits into from
Sep 19, 2023

Conversation

wlach
Copy link
Contributor

@wlach wlach commented Sep 11, 2023

Later versions in the Flask 2.x series changed the API a fair bit.
Loosen the version requirement to >= 2, which allows people to continue
using 2.0.x and 2.1.x. Add tests using 2.0.3 to be sure this continues
working. Fixes #478.

Later versions in the Flask 2.x series changed the API a fair bit.
Loosen the version requirement to >= 2, which allows people to continue
using 2.0.x and 2.1.x. Add tests using 2.0.3 to be sure this continues
working. Fixes apiflask#478.
@wlach wlach force-pushed the looser-minimum-version-required branch 2 times, most recently from f708b72 to 059004b Compare September 11, 2023 13:26
@wlach wlach force-pushed the looser-minimum-version-required branch from 059004b to 127c341 Compare September 11, 2023 13:35
@wlach wlach marked this pull request as draft September 11, 2023 14:42
@wlach
Copy link
Contributor Author

wlach commented Sep 11, 2023

There's a fair number of unit test failures because the examples depend on more recent features in flask-sqlalchemy (get_or_404, paginate). I could fix them, but I'd like to wait for guidance from @greyli before continuing.

@greyli
Copy link
Member

greyli commented Sep 12, 2023

It's unnecessary to add a separate test env for Flask 2.0.

Please create a separate requirement file for minimal dep versions, see this file for example.

@wlach wlach marked this pull request as ready for review September 15, 2023 15:37
@wlach
Copy link
Contributor Author

wlach commented Sep 15, 2023

@greyli Ok I think this is ready for review! Appreciate your support and encouragement on this

@@ -0,0 +1,5 @@
authlib==1.2.1
Copy link
Member

Choose a reason for hiding this comment

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

The purpose of the flask20 test is to test the minimum versions of the install requirements, so this file should include the following definition:

# minimum install requirements
flask==2.0.0
flask-marshmallow==0.12.0
webargs==8.3.0
flask-httpauth==4.0.0
apispec==6.0.0

# for example applications
flask-sqlalchemy==2.5.1
sqlalchemy==1.4.49
marshmallow-dataclass==8.5.0
authlib==1.2.1

And the name min (for test env, requirement file, etc.) would be more appropriate.

Copy link
Contributor Author

@wlach wlach Sep 18, 2023

Choose a reason for hiding this comment

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

That makes sense. I think I'll use the name min-versions since min might be confused with the existing minimal

@@ -86,6 +87,8 @@ def test_auto_tag_from_blueprint(app, client):
assert {'name': 'Foo'} in rv.json['tags']


@pytest.mark.skipif(flask.__version__ < '2.0.1',
reason='Depends on new behaviour introduced in Flask 2.0.1')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needed to add these skips as well if we're going to test with actual flask 2.0.0

@wlach wlach requested a review from greyli September 18, 2023 15:38
@wlach
Copy link
Contributor Author

wlach commented Sep 18, 2023

@greyli ready for another look!

Copy link
Member

@greyli greyli left a comment

Choose a reason for hiding this comment

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

LGTM

@greyli greyli merged commit 90e2f13 into apiflask:main Sep 19, 2023
16 checks passed
@greyli
Copy link
Member

greyli commented Sep 19, 2023

Merged, thanks! I'll make a new release soon.

@wlach wlach deleted the looser-minimum-version-required branch October 4, 2023 23:40
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.

minimum flask version differs between website info and github code
2 participants