- Sponsor
-
Notifications
You must be signed in to change notification settings - Fork 779
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
Use path with query string on WebSockets logs #1385
Conversation
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.
this looks good,
could you please @aminalaee take a look at it since you fixed #1290 with #1294
I'm wondering here if #1290 was legit in fact, but not sure
I was looking into the linked issue encode/starlette#1336 and since FastAPI uses Starlette under the hood, this is probably where the issue originates from. From my point of view the fix done in #1290 is legit but causes some issues due to encode/starlette#1336. |
@levrik Yes I think you're right and also changing back to The change to use |
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.
Thanks @levrik :)
Co-authored-by: Marcelo Trylesinski <marcelotryle@gmail.com>
Co-authored-by: Marcelo Trylesinski <marcelotryle@gmail.com>
I've started to use
get_path_with_query_string
for websockets logging as well to have a single place responsible for creating the path for logging.I don't know why this method wasn't used before so in case I'm missing something here, please let me know.
Fixes #1384