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

Unpin Flask and Werkzeug #313

Closed
j5awry opened this issue May 12, 2021 · 17 comments
Closed

Unpin Flask and Werkzeug #313

j5awry opened this issue May 12, 2021 · 17 comments
Labels
enhancement New feature or request

Comments

@j5awry
Copy link
Contributor

j5awry commented May 12, 2021

Is your feature request related to a problem? Please describe.
on 20210511, Flask and werkzeug released version 2.0.0. Flask had some API changes which led to breaks, but the community quickly found at least one. There's an upstream bug report in werkzeug that our tests are catching as well:

at least one is related to something report upstream to werkzeug: pallets/werkzeug#2115

We should do dependency upgrades and checks for Flask and werkzeug, run tests, see if any other errors occur in environments, and see what other issues arise in some integration tests.

Describe the solution you'd like
unpin flask and werkzeug

Describe alternatives you've considered
I refuse alternatives! As a maintainer, I maintain that right :P but, really, we should update tests and code as required for it to work. we do not want to stay pinned to lower versions as Flask and werkzeug merrily roll along.

Additional context
Pin PR: #311 311

PR to fix at least one Flask API change: #309

Original issue tracking the Flask API change: #308

@hbusul
Copy link
Contributor

hbusul commented May 12, 2021

We are using flask-restx for quite a while, I like the idea of werkzeug 2.0 with flask-restx because of the upload speed. There is improvement in werkzeug for parsing uploaded files faster and since we could not benefited from that. We used some 3rd party parser. Is there any beginner issue or clear thing to do that I could do to help speed up the transition?

@j5awry
Copy link
Contributor Author

j5awry commented May 12, 2021

@hbusul : the first place to start is running pulling the latest code for flask-restx, unpinning flask and werkzeug, and seeing what errors pop up. We also need to compare local outputs to what the Github Actions showed us:

https://github.com/python-restx/flask-restx/runs/2565726592

finally, it'd be very worthwhile if folks are willing / able to manually upgrade flask and werkzeug on testing setups and see if any other errors bubble from common use cases.

@hbusul
Copy link
Contributor

hbusul commented May 12, 2021

I will run our internal tests with latest commit(not latest release?) with werkzeug 2.0 and pinpoint the warnings and errors then. I remember tests failing for the werkzeug rc, they probably will fail for 2.0 as well.

@cedricbonhomme
Copy link

cedricbonhomme commented May 12, 2021

I am now using latest versions of the Pallets projects with flask-restx 0.4.0.
If I find issues on my project(s) I will report and eventually propose fixes.

@kannanprasanna
Copy link

Even with flask-restx 0.4.0, having Flask 2.0.0 and Werkzeug 2.0.0 causes build/runtime issues like shown below. We had to pin the Flask to 1.1.2 and Werkzeug to 1.0.1 versions to resolve this. Could this be addressed in the next release?

/flask_restx/api.py", line 20, in
from flask.helpers import _endpoint_from_view_func
ImportError: cannot import name '_endpoint_from_view_func' from 'flask.helpers'

@hbusul
Copy link
Contributor

hbusul commented May 17, 2021

@kannanprasanna they fixed it in the master branch, I guess it will be addressed in the next release.
@j5awry I ran our test suite with the master branch, tests are not failing but I'm getting some warnings:

/usr/local/lib/python3.7/site-packages/flask_restx/resource.py:46: DeprecationWarning: 'BaseResponse' is deprecated and will be removed in Werkzeug 2.1. Use 'isinstance(obj, Response)' instead.
  if isinstance(resp, BaseResponse):
/usr/local/lib/python3.7/site-packages/flask_restx/api.py:398: DeprecationWarning: 'BaseResponse' is deprecated and will be removed in Werkzeug 2.1. Use 'isinstance(obj, Response)' instead.
  if isinstance(resp, BaseResponse):
/home/broker/app/apis/ns_namespaces.py:264: DeprecationWarning: The 'attachment_filename' parameter has been renamed to 'download_name'. The old name will be removed in Flask 2.1.
/usr/local/lib/python3.7/site-packages/itsdangerous/jws.py:201: DeprecationWarning: JWS support is deprecated and will be removed in ItsDangerous 2.1. Use a dedicated JWS/JWT library such as authlib.
  super().__init__(secret_key, **kwargs)

attachment_filename getting this one where I use send_file from flask with attachment_filename that one is not related to flask-restx probably but still kept it.
Some related package versions.

Flask==2.0.0
flask-restx==0.4.1.dev0
Werkzeug==2.0.0
itsdangerous==2.0.0
Jinja2==3.0.0

@the-harpia-io
Copy link

When you are planning to release this fix?

@the-harpia-io
Copy link

Any updates?

This was referenced Jun 2, 2021
pvital added a commit to pvital/flask-restx that referenced this issue Jun 15, 2021
Signed-off-by: Paulo Vital <paulo@vital.eng.br>
pvital added a commit to perseus-github/flask-restx that referenced this issue Jun 16, 2021
Signed-off-by: Paulo Vital <paulo@vital.eng.br>
hbusul added a commit to hbusul/flask-restx that referenced this issue Jun 16, 2021
@hbusul
Copy link
Contributor

hbusul commented Jun 20, 2021

Any updates?

@aluttik
Copy link

aluttik commented Jun 25, 2021

@j5awry any updates?

j5awry added a commit that referenced this issue Jul 5, 2021
#313 don't use werkzeug, flask 2.0.0 and use Response instead of BaseResponse. Contrib: hbusul reviewer: j5awry
@j5awry
Copy link
Contributor Author

j5awry commented Jul 5, 2021

#341 has been merged. At the moment that looks like the only Flask and Werkzeug changes absolutely required.

I'm going to let it marinate on main for a little while though. If teams can pull from main and give the code a try in their environments to ensure everything is working appropriately, I'll put a release together. What I don't want is to merge this in and break a lot of people.

As you can tell, we're small and community driven (most folks making contributions are not maintainers at this point). That means response can be a bit slower when it comes to changes. I'm happy we were able to limit how many people got "broken" when flask rolled, and don't want to further break anyone when we unpin.

NOTE: If you're working in production, you should be freezing your deps. That means picking the versions of Flask and Werkzeug compatible with everything you do. we have tried to be as loosely pinned as possible and leave it to end-users. Double-edged sword there.

@Rafiot
Copy link

Rafiot commented Jul 6, 2021

Thank you @j5awry!

I just tested the updated package and it works for me (important caveat: the tool I'm using it in is very fresh, and it's not in prod yet).

For the ones reading this issue and willing to give it a try, the easiest way is as follows (simply installing the repository doesn't work as you will be missing the swagger assets):

  1. Clone python-restx
  2. Initialize your virtualenv
  3. Go to the directory you cloned python-restx into
  4. Run pip install -r requirements/test.pip
  5. Run inv assets
  6. Run pip install .
  7. Restart your flask app

@Rafiot
Copy link

Rafiot commented Jul 7, 2021

One more thing: would it make sense to make a pre-release (https://packaging.python.org/guides/distributing-packages-using-setuptools/#pre-release-versioning)?
That would probably allow more people to give it a try but it will also not be installed by default for people who don't freeze their deps.

Rafiot added a commit to Lookyloo/lookyloo that referenced this issue Jul 7, 2021
@j5awry
Copy link
Contributor Author

j5awry commented Jul 8, 2021

I think it's safe to just release and work through things. At this point, teams should have already pinned Flask and Werkzeug (hopefully). They can unprev Flask and Werkzeug in a test environment after a release. and if there are still issues, we can work through them and try to push patch releases quickly.

@Rafiot
Copy link

Rafiot commented Jul 9, 2021

This solution definitely work for me :)

@eran-gil2
Copy link

@j5awry is there an expected release date for the support of flask 2.0.1 and above?
Would really appreciate it :)

@j5awry
Copy link
Contributor Author

j5awry commented Jul 27, 2021

on version 0.5.0, flask has been unpinned

@j5awry j5awry closed this as completed Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

8 participants