fix: auth route conflict #987
Conversation
|
@VishnuSanal is attempting to deploy a commit to the sparckles Team on Vercel. A member of the Team first needs to authorize it. |
|
todo:
|
There was a problem hiding this comment.
Hey @VishnuSanal 👋
Overall , I like the approach ✨
I can do a review on the execution when it is up for review 😄
CodSpeed Performance ReportMerging #987 will not alter performanceComparing Summary
|
|
Hey @VishnuSanal , can you fetch the latest changes here?? |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Co-authored-by: Sanskar Jethi <29942790+sansyrox@users.noreply.github.com>
|
Hi all, same issue here. Any blockers for merging this? Let me know if I can help somehow! |
|
Hey @alissonpelizaro 👋 Apologies for the delay in review. I planned to review this over the weekend. If possible, could you do an initial review please? |
| HttpMethod.GET, | ||
| async_inner_handler, | ||
| injected_dependencies, | ||
| ) | ||
| else: | ||
| self.add_route(middleware_type, endpoint, inner_handler, injected_dependencies) | ||
| self.add_route(middleware_type, endpoint, HttpMethod.GET, inner_handler, injected_dependencies) |
There was a problem hiding this comment.
why is this a GET http method? Is a middleware always treated as a get request?
There was a problem hiding this comment.
This PR needs some more work and is up for grabs as @VishnuSanal is unfortunately away for some time.
sansyrox
force-pushed
the
main
branch
3 times, most recently
from
6ee78c5 to
0f16260
Compare
Description
This PR fixes #982
Summary
This PR does fix authenticated routes with different method but same path causing conflict
PR Checklist
Please ensure that:
Pre-Commit Instructions: