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

Feature: Routing overload #311

Merged
merged 1 commit into from
Jan 18, 2017
Merged

Conversation

youknowone
Copy link
Contributor

@youknowone youknowone commented Jan 18, 2017

When user specifies HTTP methods to function handlers, it automatically
will be overloaded unless they duplicate.

Example:

    # This is a new route. It works as before.
    @app.route('/overload', methods=['GET'])
    async def handler1(request):
        return text('OK1')

    # This is the exiting route but a new method. They are merged and
    # work as combined. The route will serve all of GET, POST and PUT.
    @app.route('/overload', methods=['POST', 'PUT'])
    async def handler2(request):
        return text('OK2')

    # This is the existing route and PUT method is the duplicated method.
    # It raises RouteExists.
    @app.route('/overload', methods=['PUT', 'DELETE'])
    async def handler3(request):
	return text('Duplicated')

When user specifies HTTP methods to function handlers, it automatically
will be overloaded unless they duplicate.

Example:

    # This is a new route. It works as before.
    @app.route('/overload', methods=['GET'])
    async def handler1(request):
        return text('OK1')

    # This is the exiting route but a new method. They are merged and
    # work as combined. The route will serve all of GET, POST and PUT.
    @app.route('/overload', methods=['POST', 'PUT'])
    async def handler2(request):
        return text('OK2')

    # This is the existing route and PUT method is the duplicated method.
    # It raises RouteExists.
    @app.route('/overload', methods=['PUT', 'DELETE'])
    async def handler3(request):
	return text('Duplicated')
@seemethere seemethere added this to the 0.3.0 milestone Jan 18, 2017
@seemethere seemethere merged commit e218a59 into sanic-org:master Jan 18, 2017
@seemethere
Copy link
Member

Thanks! Great job @youknowone!

@youknowone youknowone deleted the route-overload branch January 19, 2017 02:57
@youknowone
Copy link
Contributor Author

By the way, writing this patch, I had a question:
When adding a function to route, why its default methods is not just ['GET'] but all methods?
It looks weird that users must add methods=['GET'] to limit the user access only to get method.
URL endpoints are (practically) never designed to work for all of the HTTP methods. They only work for only few method.

@seemethere seemethere mentioned this pull request Jan 19, 2017
@seemethere
Copy link
Member

@youknowone That's definitely something we should look into. Maybe it would be beneficial to just default it to ['GET']

@AntonDnepr
Copy link
Contributor

Not tested this, but the implementation of this could cause this error as far as I can see:
#267 (comment)
maybe I'm wrong, but it looks the very similar way as HttpMethodView before.

@youknowone
Copy link
Contributor Author

youknowone commented Jan 21, 2017

@AntonDnepr I agree. I never thought about it. I think the mechanism itself is not harmful but its naming makes confusion. How about the class name does not end with 'view' but ends with 'route' that means it is persistent? Like HTTPMethodRoute and MethodRedirectionRoute.

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 this pull request may close these issues.

None yet

4 participants