-
Notifications
You must be signed in to change notification settings - Fork 0
chore/delete #6
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?
chore/delete #6
Conversation
Blar frog is jumping into review mode for 'chore/delete'. Let's see if this bug zapper works or if we need to fix our bug catching skills. Starting the review now! |
This PR appears to focus primarily on cleaning up code related to FastAPI's encoding and exception handling. It deletes a substantial portion of the 'jsonable_encoder' function in 'encoders.py', including the removal of complex JSON encoding logic. Additionally, it modifies a dictionary mapping in the same file, likely affecting how certain types are serialized. In 'exception_handlers.py', it enhances the HTTP exception handler to properly serialize JSON responses when exception details are dictionaries. Moreover, it removes the 'WebSocketRequestValidationError' class from 'exceptions.py'. Overall, these changes aim to streamline or refactor encoding and exception handling behavior in the FastAPI codebase. Pull Request Impact: 73 🔄 File Changes Overview
📊 Impact SummaryThis tables shows the impact of the changes in the codebase
📜 Blar InstructionsBlar Commands
Tags Explanation
|
|
||
docs/en/docs/features.md | ||
|
||
Paste it in exactly the same location but for the language you want to translate, e.g.: | ||
|
||
docs/es/docs/features.md | ||
|
||
Tip | ||
|
||
Notice that the only change in the path and file name is the language code, from en to es. | ||
|
||
If you go to your browser you will see that now the docs show your new section (the info box at the top is gone). 🎉 | ||
|
||
Now you can translate it all and see how it looks as you save the file. | ||
Don't Translate these Pages¶ | ||
|
||
🚨 Don't translate: | ||
|
||
Files under reference/ | ||
release-notes.md | ||
fastapi-people.md | ||
external-links.md | ||
newsletter.md | ||
management-tasks.md | ||
management.md | ||
contributing.md | ||
|
||
Some of these files are updated very frequently and a translation would always be behind, or they include the main content from English source files, etc. | ||
New Language¶ | ||
|
||
Let's say that you want to add translations for a language that is not yet translated, not even some pages. | ||
|
||
Let's say you want to add translations for Creole, and it's not yet there in the docs. | ||
|
||
Checking the link from above, the code for "Creole" is ht. | ||
|
||
The next step is to run the script to generate a new translation directory: | ||
|
||
|
||
|
||
|
||
Now you can check in your code editor the newly created directory docs/ht/. | ||
|
||
That command created a file docs/ht/mkdocs.yml with a simple config that inherits everything from the en version: | ||
|
||
INHERIT: ../en/mkdocs.yml | ||
|
||
Tip | ||
|
||
You could also simply create that file with those contents manually. | ||
|
||
That command also created a dummy file docs/ht/index.md for the main page, you can start by translating that one. | ||
|
||
You can continue with the previous instructions for an "Existing Language" for that process. | ||
|
||
You can make the first pull request with those two files, docs/ht/mkdocs.yml and docs/ht/index.md. 🎉 | ||
Preview the result¶ | ||
|
||
As already mentioned above, you can use the ./scripts/docs.py with the live command to preview the results (or mkdocs serve). | ||
|
||
Once you are done, you can also test it all as it would look online, including all the other languages. | ||
|
||
To do that, first build all the docs: | ||
|
||
|
||
|
||
|
||
|
||
|
||
This builds all those independent MkDocs sites for each language, combines them, and generates the final output at ./site/. | ||
|
||
Then you can serve that with the command serve: | ||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
Translation specific tips and guidelines¶ | ||
|
||
Translate only the Markdown documents (.md). Do not translate the code examples at ./docs_src. | ||
|
||
In code blocks within the Markdown document, translate comments (# a comment), but leave the rest unchanged. | ||
|
||
Do not change anything enclosed in "``" (inline code). | ||
|
||
In lines starting with /// translate only the text part after |. Leave the rest unchanged. | ||
|
||
You can translate info boxes like /// warning with for example /// warning | Achtung. But do not change the word immediately after the ///, it determines the color of the info box. | ||
|
||
Do not change the paths in links to images, code files, Markdown documents. | ||
|
||
However, when a Markdown document is translated, the #hash-parts in links to its headings may change. Update these links if possible. | ||
Search for such links in the translated document using the regex #[^# ]. | ||
Search in all documents already translated into your language for your-translated-document.md. For example VS Code has an option "Edit" -> "Find in Files". | ||
When translating a document, do not "pre-translate" #hash-parts that link to headings in untranslated documents. | ||
|
||
Was this page helpful? | ||
Previous |
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.
🟠 Warning 🎨 Design Pattern Issue
Specific guidelines for translating FastAPI docs
This PR is completely off‐track—you're pasting hundreds of lines of rendered docs into a Python source file. Per our translation guidelines, only .md
files under docs/<lang>/
get edited (and inside code blocks you only translate comments). Revert these changes, restore the original encoders.py
, and move any translation work into the proper Markdown files.
We used the following considerations to provide context for this review — see them here
Reply if you have any questions or let me know if I missed something.
Don't forget to react with a 👍 or 👎 to the comments made by Blar to help us improve.
docs/es/docs/features.md | ||
|
||
Tip | ||
|
||
Notice that the only change in the path and file name is the language code, from en to es. | ||
|
||
If you go to your browser you will see that now the docs show your new section (the info box at the top is gone). 🎉 | ||
|
||
Now you can translate it all and see how it looks as you save the file. | ||
Don't Translate these Pages¶ | ||
|
||
🚨 Don't translate: | ||
|
||
Files under reference/ | ||
release-notes.md | ||
fastapi-people.md | ||
external-links.md | ||
newsletter.md | ||
management-tasks.md | ||
management.md | ||
contributing.md | ||
|
||
Some of these files are updated very frequently and a translation would always be behind, or they include the main content from English source files, etc. | ||
New Language¶ | ||
|
||
Let's say that you want to add translations for a language that is not yet translated, not even some pages. | ||
|
||
Let's say you want to add translations for Creole, and it's not yet there in the docs. | ||
|
||
Checking the link from above, the code for "Creole" is ht. | ||
|
||
The next step is to run the script to generate a new translation directory: | ||
|
||
|
||
|
||
|
||
Now you can check in your code editor the newly created directory docs/ht/. | ||
|
||
That command created a file docs/ht/mkdocs.yml with a simple config that inherits everything from the en version: | ||
|
||
INHERIT: ../en/mkdocs.yml | ||
|
||
Tip | ||
|
||
You could also simply create that file with those contents manually. | ||
|
||
That command also created a dummy file docs/ht/index.md for the main page, you can start by translating that one. | ||
|
||
You can continue with the previous instructions for an "Existing Language" for that process. | ||
|
||
You can make the first pull request with those two files, docs/ht/mkdocs.yml and docs/ht/index.md. 🎉 | ||
Preview the result¶ | ||
|
||
As already mentioned above, you can use the ./scripts/docs.py with the live command to preview the results (or mkdocs serve). | ||
|
||
Once you are done, you can also test it all as it would look online, including all the other languages. | ||
|
||
To do that, first build all the docs: | ||
|
||
|
||
|
||
|
||
|
||
|
||
This builds all those independent MkDocs sites for each language, combines them, and generates the final output at ./site/. | ||
|
||
Then you can serve that with the command serve: | ||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
Translation specific tips and guidelines¶ | ||
|
||
Translate only the Markdown documents (.md). Do not translate the code examples at ./docs_src. | ||
|
||
In code blocks within the Markdown document, translate comments (# a comment), but leave the rest unchanged. | ||
|
||
Do not change anything enclosed in "``" (inline code). | ||
|
||
In lines starting with /// translate only the text part after |. Leave the rest unchanged. | ||
|
||
You can translate info boxes like /// warning with for example /// warning | Achtung. But do not change the word immediately after the ///, it determines the color of the info box. | ||
|
||
Do not change the paths in links to images, code files, Markdown documents. | ||
|
||
However, when a Markdown document is translated, the #hash-parts in links to its headings may change. Update these links if possible. | ||
Search for such links in the translated document using the regex #[^# ]. | ||
Search in all documents already translated into your language for your-translated-document.md. For example VS Code has an option "Edit" -> "Find in Files". | ||
When translating a document, do not "pre-translate" #hash-parts that link to headings in untranslated documents. | ||
|
||
Was this page helpful? | ||
Previous | ||
Help FastAPI - Get Help | ||
Next | ||
Full Stack FastAPI Template | ||
The FastAPI trademark is owned by @tiangolo and is registered in the US and across other regions | ||
Made with Material for MkDocs |
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.
🔴 Error 🐛 Bug
HTML content inserted into encoders.py breaks Python syntax.
Issue Explanation
- File at
blarApp/fastapi/fastapi/encoders.py
contains non-Python text after a lambda assignment. - Lines starting with "Skip to content" and "Follow FastAPI on LinkedIn to stay updated" are invalid in Python.
- The expression
lambda td: td.total_
is incomplete and followed by HTML, causing a syntax error on import.
datetime.timedelta: lambda td: td.total_
Skip to content
Follow FastAPI on LinkedIn to stay updated
Reply if you have any questions or let me know if I missed something.
Don't forget to react with a 👍 or 👎 to the comments made by Blar to help us improve.
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.
could u help me understand why this issue is a problem?
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.
Absolutely, let me clarify why this is a problem.
The issue is that your file, encoders.py
, has some unexpected non-Python text inserted directly into the code. Here’s what’s wrong:
-
Python source files must only contain valid Python code (and possibly comments or docstrings).
-
In your case, after a line with lambda assignment, there are lines like:
datetime.timedelta: lambda td: td.total_ Skip to content Follow FastAPI on LinkedIn to stay updated
-
The text
Skip to content
andFollow FastAPI on LinkedIn to stay updated
is not valid Python—it looks like HTML or a pasted banner from a web page. -
This extra text will cause a syntax error when Python tries to parse the file, making it impossible to run or import.
Additionally, the lambda expression is incomplete:
lambda td: td.total_
This doesn’t look like a complete method—it’s missing something (probably should be td.total_seconds()
).
Quick summary:
Having random non-code text inside a Python file breaks the file, causing it not to run, and the lambda itself is also incomplete.
If you need help fixing it, let me know!
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.
lets make a good rule for our wiki to make sure this never happens again
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.
Great idea! Here’s a clear rule you can add to your wiki to prevent this type of issue in the future:
📜 Python Source File Hygiene Rule
Rule:
All Python files in the project must contain only valid Python code, comments, or documentation strings.
External content such as HTML, banners, web labels, or unrelated text must never be inserted into Python source code.
Why:
Non-Python content in source files will break parsing and execution, causing syntax errors and blocking imports.
How to Avoid:
- Do not paste code directly from web pages without checking for hidden or non-code lines.
- Review any changes before committing to ensure only valid Python code is present.
- Use an editor or IDE that highlights syntax errors.
Example of what NOT to do:
def my_function():
print("Hello, world!")
Skip to content
Follow us on LinkedIn
Would you like me to suggest a checklist or pre-commit hook to help enforce this as well?
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.
🟠 Warning 🐛 Bug
jsonable_encoder
function definition is removed, causing NameError on existing calls.
Issue Explanation
- The
jsonable_encoder
function definition is fully removed (seeMerged file: jsonable_encoder node marked DELETED
). - Existing calls to
jsonable_encoder
now reference an undefined name. - Runtime errors (NameError) will occur when such calls execute.
- No alternative implementation for
jsonable_encoder
is present.
No information found
Reply if you have any questions or let me know if I missed something.
Don't forget to react with a 👍 or 👎 to the comments made by Blar to help us improve.
class WebSocketRequestValidationError(ValidationException): | ||
pass | ||
|
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.
🔴 Error 🐛 Bug
Signature in exception_handlers.py references removed class WebSocketRequestValidationError
, causing a NameError at import.
Issue Explanation
- In
blarApp/fastapi/fastapi/exception_handlers.py
, the functionwebsocket_request_validation_exception_handler
declares the parameterexc: WebSocketRequestValidationError
. - The class
WebSocketRequestValidationError
has been deleted fromfastapi/exceptions.py
. - The annotation of a non-existent class raises a NameError when the module is loaded.
async def websocket_request_validation_exception_handler(
websocket: WebSocket, exc: WebSocketRequestValidationError
) -> None:
Reply if you have any questions or let me know if I missed something.
Don't forget to react with a 👍 or 👎 to the comments made by Blar to help us improve.
Review's done! 🚀 Check out the feedback and let me know if you need anything! – Blar |
Looks like your code is as disciplined as an unruly toddler after a sugar rush. Did you even run the tests or just hope for the best with this chaos? |
No description provided.