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

fix: allows user to set example(s) value(s) when using Form and/or multiple Body parameters #5297

Closed

Conversation

Thytu
Copy link

@Thytu Thytu commented Aug 23, 2022

This would allow the user to use and display example(s) value(s) when using Form (or multiple Body param).

Explication: f.field_info.extra was empty, thus when calling create_response_field the return value also has en empty extra field.

Note : I know this is not a perfect fix as it does not handle every cases (and is hard-coded) but this would fix the issue while giving time for a developing a better fix to come latter.

Better fix for latter : When creating the field_info in get_param_field, we should determine which value to put into the extra field.

Closes #5249

@github-actions
Copy link
Contributor

📝 Docs preview for commit 465fa70 at: https://6304a9f3f932165c1c5f3902--fastapi.netlify.app

@codecov
Copy link

codecov bot commented Aug 23, 2022

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (cf73051) 100.00% compared to head (fc7a4e3) 100.00%.

❗ Current head fc7a4e3 differs from pull request most recent head 1c83608. Consider uploading reports for the commit 1c83608 to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #5297   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          540       540           
  Lines        13969     13955   -14     
=========================================
- Hits         13969     13955   -14     
Impacted Files Coverage Δ
fastapi/dependencies/utils.py 100.00% <100.00%> (ø)
tests/test_schema_extra_examples.py 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@github-actions
Copy link
Contributor

📝 Docs preview for commit 54c3cce at: https://6304ceb57221bc00af3fdb7f--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit ddb8c97 at: https://6304d21be4d2a200b6672bf8--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit a6927c3 at: https://6304e7d434435108f8ec8054--fastapi.netlify.app

@Thytu Thytu force-pushed the fix/move-example-value-into-field_info-extra branch from ad579ea to 082da0a Compare August 23, 2022 14:55
@github-actions
Copy link
Contributor

📝 Docs preview for commit 082da0a at: https://6304eb466af6460e89830243--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit 6776a87 at: https://6304ee5e40f0bf09bcfcc3c2--fastapi.netlify.app

@Thytu
Copy link
Author

Thytu commented Aug 24, 2022

@JarroVGIT is the test coverage KO?

It's running since yesterday 🫤

@JarroVGIT
Copy link
Contributor

That is weird, I am not the maintainer of this project (@tiangolo) is, maybe it’s just stuck and a nonsense commit would force it to restart?

@github-actions
Copy link
Contributor

📝 Docs preview for commit fc7a4e3 at: https://63514fea133908387a64036b--fastapi.netlify.app

@github-actions
Copy link
Contributor

github-actions bot commented Nov 4, 2022

📝 Docs preview for commit 516ac3e at: https://63651605f1f3d2004eef785b--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit 0baaca7 at: https://637670959ffd0c114a4849bf--fastapi.netlify.app

@Thytu
Copy link
Author

Thytu commented Nov 22, 2022

Legend says that one day, this PR will be either merger or closed, but no one know which and when 🫠

@github-actions
Copy link
Contributor

📝 Docs preview for commit 010df4a at: https://637e24626bc32f4503acf9b7--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit e11412a at: https://638366b289f64174f6e8f97c--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit 2de9b27 at: https://6383702d603c3c69a92c000b--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit 75b10ac at: https://63847192d901c52783138872--fastapi.netlify.app

@github-actions
Copy link
Contributor

github-actions bot commented Dec 5, 2022

📝 Docs preview for commit dc5f4ec at: https://638e2b21fc1d9854575661c1--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit 2f5bdc4 at: https://639ce566a12b8e05b99de518--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit c0c8198 at: https://639ed7ce04318b4f66823fd5--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit 1c83608 at: https://64254c5d8bd61a4016f6c349--fastapi.netlify.app

@Thytu
Copy link
Author

Thytu commented Mar 30, 2023

I'm stubborn (and working on a fork since Aug 2022 🤡)

@Thytu
Copy link
Author

Thytu commented Sep 21, 2023

Cleaning my PR history (takes WAY too long to be merged...)

@Thytu Thytu closed this Sep 21, 2023
@JarroVGIT
Copy link
Contributor

@tiangolo this is a great example on why there needs to be a better format for managing this project. You ask for help specifically on this project, but if you don’t even comment on active code contributions, people will move away.

@tiangolo
Copy link
Owner

Thanks for the input @JarroVGIT, I'm trying to work on the things that can make the biggest impact, switching back and forth in reviewing things and the others that are already planned. In this particular case, I have been wanting to support Pydantic models for Form, Query, and others, but I haven't been able to see how that would work and if it would be feasible or not, so I didn't (and still don't) know if this would be the approach to go or not, so I didn't comment before, as I don't really have something valuable to comment yet.

@Thytu
Copy link
Author

Thytu commented Sep 28, 2023

@tiangolo I have no doubt that you're are prioritizing the PR / Issues than can produce the biggest impact, but @JarroVGIT's point is still valid.

You should consider changing the current format of management for this project for one than could result in a faster iteration rate and thus biggest impact.

The current working do results in people moving away and in our case considering switching FastAPI for something else.
FastAPI is still the most pleasant way to write API in python, you did a great job on that, but those little bug/issues results in a non-"prod proof" package.

I hope you'll consider those elements, either way well done for the work accomplished.

@tiangolo
Copy link
Owner

Thanks for the feedback. I'll try to make things more transparent, I think probably a tentative roadmap will help (although I'm afraid people would be disappointed if they find out I discarded/changed later some of the ideas in that roadmap). That could help understanding what is planned and what will come in features.

On the other hand, meanwhile, something everyone could help is reviewing existing PRs (@JarroVGIT did it here, thanks!), in particular, checking they have tests, and that the code in the PR solved the problem.

For example, there are many PRs, you could help me asking for them.

There's more info and instructions here: https://fastapi.tiangolo.com/help-fastapi/#review-pull-requests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to have multiple str in body with an example for each of these str (while typing those value as string)
4 participants