-
-
Notifications
You must be signed in to change notification settings - Fork 772
✨ Add support for standard tracebacks via the env TYPER_STANDARD_TRACEBACK
#1299
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?
✨ Add support for standard tracebacks via the env TYPER_STANDARD_TRACEBACK
#1299
Conversation
Keeps support fo `_TYPER_STANDARD_TRACEBACK` but notes as deprecated because: - It is non-standard for an advertised env to start with an underscore. - Environment variables that start with an underscore have been observed as causing issues in some execution environments (e.g. with AWS Lambda). See: fastapi#1284
📝 Docs preview for commit b77ece1 at: https://11faf57b.typertiangolo.pages.dev Modified Pages |
TYPER_STANDARD_TRACEBACK
TYPER_STANDARD_TRACEBACK
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.
As discussed, this PR adds the option to set an env var TYPER_STANDARD_TRACEBACK
while still supporting the old var _TYPER_STANDARD_TRACEBACK
too. We need to support both because we don't want to break existing workflows, but that does complicate the code slightly. I think that's an acceptable trade-off to ensure this variable can be properly set in AWS Lambda etc.
standard_traceback = os.getenv("_TYPER_STANDARD_TRACEBACK") | ||
standard_traceback = os.getenv( | ||
"TYPER_STANDARD_TRACEBACK", os.getenv("_TYPER_STANDARD_TRACEBACK") | ||
) |
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.
It's worth pointing out that this implementation introduces a priority: if both of these variables are set in the environment, then only TYPER_STANDARD_TRACEBACK
will be retrieved & used. I think that's fine though, and it would be an extreme niche case if TYPER_STANDARD_TRACEBACK
was set to 0
for instance and _TYPER_STANDARD_TRACEBACK
to 1
🤷♀️
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.
I had the same thought. However, I noticed that it's not possible to turn this functionality off by setting the env to any non-empty value:
Lines 68 to 73 in 0b6a405
standard_traceback = os.getenv("_TYPER_STANDARD_TRACEBACK") | |
if ( | |
standard_traceback | |
or not exception_config | |
or not exception_config.pretty_exceptions_enable | |
): |
standard_traceback = "0" -> truthy
).
The consequence is that a disagreement can only happen when one variable is set to empty string, and another set to a value. In this case, did the user mean empty string to signal off, or just "unset"?
I think the new env overriding the legacy is acceptable but it's debatable.
I considered changing to support 0 -> off
but I didn't want to increase the scope (and it could be argued that it is a breaking change - but it probably more of a bug fix).
I will leave this PR for a final review by Tiangolo 🙏 |
This PR allows standard tracebacks to be enabled using the environment variable:
TYPER_STANDARD_TRACEBACK
.The work is a follow up to the discussion with @svlandeg: #1284.
The existing environment variable
_TYPER_STANDARD_TRACEBACK
is retained but referred to as "deprecated" because:Manual testing with:
Produces expected results: