Warn or raise when overriding an endpoint #570

Closed
SimonSapin opened this Issue Jul 29, 2012 · 5 comments

Comments

Projects
None yet
4 participants
Contributor

SimonSapin commented Jul 29, 2012

It is apparently a common mistake to make multiple/distinct view functions sharing the same endpoint, the last overriding the earlier ones in the app.view_functions dict.

With a decorator: http://stackoverflow.com/questions/11064263/flask-custom-decorator-breaks-the-routing
With class-based views: http://stackoverflow.com/questions/11700698/flask-route-query-parameter

Is there an use case to using app.route() or app.add_url_rule() with an existing endpoint and overriding it? I think it is more often a mistake than not. I suggest that such cases issue a warning or even raise an exception, in order to catch the mistake early. In uncommon scenarios where overriding is actually desired, the user can always use del app.view_functions[endpoint] before registering the new function.

(I can provide a patch, but I’d like to discuss the idea first.)

+1, seems like a good idea to me. I'd appreciate it as a user. Happy to write a patch or work with you on one, Simon.

Contributor

SimonSapin commented Sep 18, 2012

Wow, I had forgotten about this. @yoavshapira , go ahead and write the patch if you like, and mention this issue’s number in the pull request. I can review it then, but unfortunately I can’t help more than that in getting it merged :)

No problem, will do.

Do people have a preference regarding raising an exception, logging a warning, or returning False (or another value maybe) from the add_url_rule method? Whichever you prefer, I'll do and send in a pull request.

Here's the "log a warning" approach, with a matching unit test: YoavShapira/flask@859f091

mitsuhiko closed this in 661ee54 Oct 7, 2012

Contributor

mrjoes commented Jun 14, 2013

This fix broke compatibility with class-based views/bound methods.

Simplified flask-admin case:

from flask import Flask, Blueprint

app = Flask(__name__)

class Dummy(object):
    def __init__(self, app, name):
        self.name = name

        bp = Blueprint(name, __name__)
        bp.add_url_rule('/', 'view', self.view)
        bp.add_url_rule('/<test>/', 'view', self.view)

        app.register_blueprint(bp)

    def view(self, test=None):
        return 'hi'

obj1 = Dummy(app, 'test')
app.run(port=8002)

In Flask 0.9 it works as expected, but in Flask 0.10 it does not.

Cased by is check:

> obj.view is obj.view
False
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment