-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
♻️ Simplify code in fastapi/routing.py
#14232
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
base: master
Are you sure you want to change the base?
Conversation
8ee5297 to
553faa0
Compare
| raise http_error from e | ||
|
|
||
| # Solve dependencies and run path operation function, auto-closing dependencies | ||
| errors: List[Any] = [] |
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.
variable is overwritten by errors = solved_result.errors a bit lower
6007760 to
dba23ba
Compare
fastapi/routing.py
Outdated
| is_body_form = body_field and isinstance( | ||
| body_field.field_info, (params.Form, temp_pydantic_v1_params.Form) | ||
| ) | ||
| if is_body_form: |
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.
doesn't change internal rules, just simplify code removing nesting
| raise RequestValidationError(_normalize_errors(errors), body=body) | ||
|
|
||
| # Return response | ||
| assert response |
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.
assert response was used to avoid forget response assignment inside if-else-branches, not required in proposed changes
1424f21 to
3365050
Compare
f5dc1bf to
030f014
Compare
|
@YuriiMotov hi! I removed last commit (which extracted So PR is ready, may you please remove |
fastapi.routing.py
fastapi.routing.pyfastapi/routing.py
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.
LGTM!
With these changes code is more readable. Changes don't affect final result
@bokshitsky, thank you!
TLDR: just simplify internal logic and code a bit, doesn't change framework behaviour
split in in 5 commits:
remove unused variable initialization
errors: List[Any] = []simplify code: remove nested code, using early exit pattern
write:
if remains exactly the old behaviour, but
a) looks more clear (right now looks like
status_code if status_code else solved_result.response.status_codeis default value, but actually in the end default value issolved_result.response.status_codeb) it doesn't make any speed effect, but new version use less comparison and assignment operations
responseinitialization