Skip to content
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

Changes to Blueprints application breaks multiply applied routes!!! #1742

Closed
autumnjolitz opened this issue Dec 27, 2019 · 5 comments · Fixed by #1764
Closed

Changes to Blueprints application breaks multiply applied routes!!! #1742

autumnjolitz opened this issue Dec 27, 2019 · 5 comments · Fixed by #1764

Comments

@autumnjolitz
Copy link

Consider the following pattern:

@app.get("/url1")
@app.get("/url2")
async def handle_multiple(request):
     return text("some response")

The recent change in #1690 breaks that pattern.

It's a common pattern to use multiple @blueprint.get(...) and @app.get(...) to handle routes that have common behavior with slight variations, like a login process.

The only alternative I can think of is splitting out each of these routes into distinct rump functions that literally forward onto a common function, which inflates from one line declarations to three lines plus the constant common route handler.

@harshanarayana
Copy link
Contributor

@benjolitz

The original change in #1690 was done in order to account for an old standing open issue that actually needed fixing. I missed this case when implementing the change that seems to have broken the case of multiple routes being bound to the same handler method.

Let me see what can be done to address this while still being able to handle the original request from #37

@autumnjolitz
Copy link
Author

@harshanarayana okay. wrong username for me btw. ;)

@harshanarayana
Copy link
Contributor

@autumnjolitz oh. Sorry. That explains why the auto complete wasn't working last night. 😂 sorry about the mistake

@ashleysommer
Copy link
Member

ashleysommer commented Jan 8, 2020

Hi @autumnjolitz
Until we get a proper fix for this, can you try this workaround?

async def handle_multiple(request):
     return text("some response")
names1, handler1 = app.get("/url1")(handle_multiple)
names2, handler2 = app.get("/url2")(handle_multiple)

ashleysommer added a commit to ashleysommer/sanic that referenced this issue Jan 10, 2020
yunstanford pushed a commit that referenced this issue Jan 11, 2020
* Allow route decorators to stack up without causing a function signature inspection crash
Fix #1742

* Apply fix to @websocket routes docorator too
Add test for double-stacked websocket decorator
remove introduction of new variable in route wrapper, extend routes in-place.
Add explanation of why a handler will be a tuple in the case of a double-stacked route decorator
@autumnjolitz
Copy link
Author

Thank you @ashleysommer !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants