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

endpoint resolution by module #54

Closed
wants to merge 2 commits into from
Closed

endpoint resolution by module #54

wants to merge 2 commits into from

Conversation

WnP
Copy link

@WnP WnP commented May 14, 2014

fix #53
suppose you have two modules:

main.py

from klein import Klein

app = Klein()

@app.route('/')
def home(request):
    return 'hello from main home'

and in a second module, you defined another home for a different home:

blog.py

from main import app

@app.route('/blog_home')
def home(request):
    return 'hello from blog home'

in this case if you point your browser -or something else- to / or /blog_home in both case it will return hello from blog home

this patch fix this issue, hope you'll like it!

@WnP WnP mentioned this pull request May 14, 2014
@WnP
Copy link
Author

WnP commented May 14, 2014

the following example show the fullyQualifiedName limits with decorator:

from twisted.python.reflect import fullyQualifiedName


def name_deco(f):
    def wrapper(*args, **kwargs):
        print "from decorator: " + fullyQualifiedName(f)
        return f(*args, **kwargs)
    return wrapper


class Foo(object):

    @name_deco
    def bar(self):
        pass


@name_deco
def bar():
    pass


print fullyQualifiedName(Foo.bar)
print fullyQualifiedName(bar)

# call to trig decorator
f = Foo()
f.bar()
bar()

the above script output:

__main__.Foo.wrapper
__main__.wrapper
from decorator: __main__.bar
from decorator: __main__.bar

using decorator fullyQualifiedName can't make deference between the method Foo.bar and the bar function

@glyph
Copy link
Member

glyph commented Sep 12, 2014

Hi @WnP - thanks for spotting this issue. Sorry that you had to wait so long for feedback on it.

I'm inclined to merge it, but it doesn't look like the test coverage you've changed really clearly explains what the issue was or tests for it directly.

Would you mind adding a test which fails on trunk, in much the same way that the example in the description of the PR fails?

@glyph
Copy link
Member

glyph commented Jan 23, 2015

@WnP Ping?

@WnP
Copy link
Author

WnP commented Jan 23, 2015

Hi @glyph
I will do that soon, but right now I don't have so much time, give me one more week please

@glyph
Copy link
Member

glyph commented Jan 23, 2015

@WnP No worries! Just wondering if you had lost interest entirely. You can have as many weeks or months to finish as you need :).

@glyph
Copy link
Member

glyph commented Aug 29, 2015

@WnP - ping - just reminding you that it's been more than a week :).

@glyph
Copy link
Member

glyph commented Oct 13, 2015

@WnP - any word?

@glyph
Copy link
Member

glyph commented Jul 19, 2016

Since the submitter has apparently idled out I'm going to close this for now. But if someone wanted to step up and take over this code I would be happy to re-review it.

@glyph glyph closed this Jul 19, 2016
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

2 participants