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

#313 don't use werkzeug, flask 2.0.0 and use Response instead of BaseResponse #341

Merged
merged 1 commit into from
Jul 5, 2021

Conversation

hbusul
Copy link
Contributor

@hbusul hbusul commented Jun 16, 2021

This code imports BaseResponse if version of werkzeug is less than 2.0.0 otherwise it imports Response as suggested.
In addition to that, it forbids werkzeug and flask version 2.0.0 because description of HttpException becomes None and it fails the tests. This behavior is fixed in version 2.0.1, so users still can use version 2.0.1. @j5awry I know changes are short but could you take a look at it? Or, is the repo waiting for more people to report breaking changes? In that case, could you let us know when that period will end?

@silasary
Copy link

Can we merge this?

@Rafiot Rafiot mentioned this pull request Jun 28, 2021
@Rafiot
Copy link

Rafiot commented Jul 1, 2021

Is there anything we could do to help getting this PR merged (and a new release packaged)? I'd really love to be able to use restx without reverting the upgrade to flask 2 in a couple of projects.

@hbusul
Copy link
Contributor Author

hbusul commented Jul 1, 2021

I can't and I shouldn't. It would be nice someone with deeper understanding of the repo reviewed but no response so far. In the times of covid19, I'm kinda hesitant to pinging people frequently but maybe we should ping some other maintainer?

@codecov
Copy link

codecov bot commented Jul 5, 2021

Codecov Report

Merging #341 (acc4dc9) into master (af807aa) will increase coverage by 0.08%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #341      +/-   ##
==========================================
+ Coverage   96.78%   96.86%   +0.08%     
==========================================
  Files          20       20              
  Lines        2734     2740       +6     
==========================================
+ Hits         2646     2654       +8     
+ Misses         88       86       -2     
Impacted Files Coverage Δ
flask_restx/api.py 96.75% <100.00%> (+0.48%) ⬆️
flask_restx/resource.py 98.07% <100.00%> (+0.11%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af807aa...acc4dc9. Read the comment docs.

@j5awry j5awry merged commit 8a01bc7 into python-restx:master Jul 5, 2021
@j5awry
Copy link
Contributor

j5awry commented Jul 5, 2021

@hbusul Thanks for the work on this. I did some checks, ran the tests, and tried it with the local test projects, and everything looks good. I've merged this in.

And thank you for not pinging too often. I'm the only maintainer working on things ATM, and I've been absolutely swamped. also doesn't help I'm not a web-developer anymore, and not using the framework at work.

@j5awry j5awry mentioned this pull request Jul 5, 2021
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.

4 participants