-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add Flask example and tests #2
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First, thank you so much for putting so much effort into showcasing Flask's capabilities to provide web APIs.
A couple of remarks:
- README.md is missing
- requirements.txt is missing
- it would be nice to have some curl commands for copy pasting at the top level README to play with the API
Additionally, I have left some inline comments.
Btw, what is the reason to not support PUT
?
Could you please explain the indirect=True
parameter for parametrize
? I had a look at pytest
s docs, but I am still puzzled.
Disclaimer, I don't work a lot with REST API.
I used curl
to query the API, and looks good from my side!
Thanks for the review, I just added the missing README.md and requirements.txt/in files, and moved error messages and category list to separate variables.
Sure, I will add it later.
The PUT for updating a resource will be duplicate most of the
It will pass the items in the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some more minor comments 👍
This is a basic example for Flask and some universal tests for all the examples. The app generally follows the design in #1, the only difference is that I remove the
age
field.After it's merged, I will start to set up the repo (tox, pre-commit hook, CI, etc.).