Feat before request #1125
Feat before request #1125
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Hey @tengfeisky 👋
Thanks for the PR :D I have some comments. Could you have a look?
|
|
||
| @app.get("/sync/global/middlewares/with_request") | ||
| def sync_global_middlewares_with_request(request: Request): | ||
| print(request.headers) |
|
|
||
| @app.get("/async/global/middlewares/with_request") | ||
| def sync_global_middlewares_with_request(request: Request): | ||
| print(request.headers) |
| if let Ok(tuple) = function_args.downcast::<PyTuple>(py) { | ||
| handler.call(tuple, Some(kwargs)) | ||
| } else { | ||
| handler.call((function_args,), Some(kwargs)) | ||
| } |
There was a problem hiding this comment.
can you please explain this line of code to me?
There was a problem hiding this comment.
In Rust, this syntax (function_args,) creates a nested tuple in Python, similar to ((arg1,arg2),).
| @@ -1,5 +1,6 @@ | |||
| use crate::executors::{ | |||
| execute_http_function, execute_middleware_function, execute_startup_handler, | |||
| execute_http_function, execute_middleware_after_request, execute_middleware_function, | |||
There was a problem hiding this comment.
I think we can do slightly better here.
i.e. execute_middleware_after_request and execute_middleware_function are not correct nomenclature imo.
There was a problem hiding this comment.
What is your suggestion then?
![recurseml[bot]](https://avatars.githubusercontent.com/in/1054195?s=60&v=4){kind=small}
|
|
||
|
|
||
| @app.get("/async/global/middlewares/with_request") | ||
| def sync_global_middlewares_with_request(request: Request): |
There was a problem hiding this comment.
Function name duplication: This function has the same name as another handler defined on line 150. In Python, function names must be unique within the same scope. The second definition will override the first one, causing both routes ('/sync/global/middlewares/with_request' and '/async/global/middlewares/with_request') to use the same handler. This will lead to unexpected behavior where both routes execute the same code. Each route handler should have a unique name.
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
|
😱 Found 1 issue. Time to roll up your sleeves! 😱 |
2 participants
Description
This PR adds request parameter to after_request middleware
Summary
This PR adds the request parameter to the after_request middleware function to enable proper propagation of trace_id between request and response headers. This change supports distributed tracing functionality by ensuring that the trace_id remains consistent throughout the request lifecycle and can be properly passed to downstream services.
PR Checklist
Please ensure that:
Pre-Commit Instructions: