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

A way to structure klein apps into (reusable) modules #73

Closed
tehasdf opened this issue Apr 13, 2015 · 30 comments
Closed

A way to structure klein apps into (reusable) modules #73

tehasdf opened this issue Apr 13, 2015 · 30 comments

Comments

@tehasdf
Copy link

tehasdf commented Apr 13, 2015

nb. the pattern I'm showing here is basically flask's blueprints

It seems that currently Klein doesn't have any builtin way to help factoring out view functions to separate modules.

For example, if you had the following app:

app = Klein()

@app.route('/users')
def list_all_users(request):
    return 'all users'

then you could divide it into two modules like this:

# main.py
app = Klein()
# users.py - version 1
from main import app

@app.route('/users')
# ...

This is bad because it leads to several problems, the most immediate being circular import issues, but also this is only a separate module technically, not logically: you can't really re-use that module with another app, or test it in isolation.

An easy way to help that would be to use dependency injection:

# main.py
from users import add_users_routes

app = Klein()
add_users_routes(app)
# users.py - version 2

def list_all_users(request): 
    return 'all_users'

def add_users_routes(app):
    app.route('/users')(list_all_users)

Now users is a separate logical module, but it's a bit awkward, with the central add_users_routes function. We could use Klein's .resource() to help that:

# users.py - version 3
users = Klein()

@users.route('/')
def list_all_users(request):
    return 'all users'

def add_users_routes(app):
    @app.route('/users', branch=True)
    def branch_to_users(request):
        return users.resource()

This is already pretty nice, could maybe use some helpers so that you wouldn't need to implement that branch function, but is reusable and possible to test in isolation. The problem however is, routing to a Klein resource returns control to twisted.web, which then calls .render() on the nested resource, which causes a werkzeug map bind, etc - it's a performance hit. A simple hello-world-grade benchmark shown that a root Klein can serve ~2200 requests per second, routing one branch deep: ~1700, two branches deep: ~1400 (experience with flask's blueprints shows that 2 levels of nesting are enough in practice)

I'm aware Klein is a web framework, and web applications aren't typically constrained by strict real-time requirements, and they're often easy to scale, but I think Klein needs some guideline on structuring applications, and while we're at it, might as well make it fast :)

Basically I think we want the syntax of users.py version 2, with the performance of version 3. Here's how flask does it (translated to Klein):

# main.py
from users import users_blueprint
app = Klein()
users_blueprint.register(app, url_prefix='/users')
# users.py
users_blueprint = Blueprint()

@users_blueprint.route('/')
# ...

The Blueprint is simply a container that records all @route() calls it was used for, and does them on the app when .register is called. This makes it only a setup-time thing, with no runtime (performance) effects.

I've put together a proof-of-concept implementation for reference, see https://github.com/tehasdf/klein/tree/blueprints (here's an example.rst in the repo)

@wachpwnski
Copy link

This is definitely something that should be looked into being implemented on Klein's main. We need to think of a what you would call a twisted blueprint though.

@gsemet
Copy link

gsemet commented May 6, 2015

Here is how I do:

File: app/routes/init.py

from klein import Klein

...

app = Klein()

...

# injecting all *.py file other than __init__.py in this file
modules = glob.glob(os.path.dirname(__file__) + "/*.py")
__all__ = [os.path.basename(f)[:-3] for f in modules]

from app.routes.api import *

From now on, I just have to add file such as:

...

from approutes import app

@app.route(API_DOC_ENTRY_POINT + '/<path:path1>', methods=['GET'])
def api_doc1(request, path1):
    return serve_url(path1, Config().frontend.doc_full_path)

....

This is hacky, but it works well :)

@sunkencity
Copy link

it is possible to mount several klein apps in a tree by having another twisted web root and then mounting them under different paths with root.putChild("path", klein_instance.resource())

@glyph
Copy link
Member

glyph commented Oct 8, 2016

This is already possible, and is used extensively on Mimic.

I thought that we even had an example of doing this in the documentation, but apparently you have to sort of infer it from cobbling together a combination of "return anything", "static files" and "non-global state".

The way that you do it is using branch routes, and bound apps, like so:

from klein import route, run, Klein

class UsersApp(object):
    routes = Klein()
    @routes.route("/<string:username>")
    def an_user(self, request, username):
        request.setHeader('content-type', 'text/plain')
        return 'A User Named "{username}".'.format(username=username)

class MathApp(object):
    mathroutes = Klein()
    @mathroutes.route("/add/<int:a>/<int:b>")
    def add(self, request, a, b):
        return str(a + b)


@route("/users", branch=True)
def users_app(request):
    return UsersApp().routes.resource()

@route("/math", branch=True)
def math_app(request):
    return MathApp().mathroutes.resource()

@route("/")
def hi(request):
    return "Hi"

run("localhost", 8123)

Here we have two apps - "math" and "users", each of which is "mounted" into the URL hierarchy by the top-level branch route. So you can go to /users/janet or you can go to /math/add/3/4.

This ticket is asking for an implementation, which I don't think we need, so I'm going to close it. I do think there's a valid issue here though, which is for there to be some clear documentation of this pattern, but I'm not sure how to articulate it in terms of a useful example, so I'm not opening that right away. I would definitely appreciate it if someone else could open it though!

cc @wsanchez

@glyph glyph closed this as completed Oct 8, 2016
@tehasdf
Copy link
Author

tehasdf commented Oct 8, 2016

@glyph, yes, now that I think about it, it's not really needed.

It is useful in flask, because while in flask you can also return wsgi apps from viewfuncs (much like you do in this example by returning klein resources), you usually don't want to have multiple flask apps (because an app is stateful, holding things like a db connection). This means you want to have modularity while still keeping a single app object (hence, blueprints). Klein usually doesn't store anything on the app object, so modularity via multiple apps like that is fine. (well, the performance hit I mentioned before still exists)

@wachpwnski
Copy link

wachpwnski commented Oct 13, 2016

Doesn't using branch=True have a performance impact in this case though?

I also get hanging on redirects using glyph's method. If you create a "UsersApplicaiton" that has '/' routes in it, it freezes on redirects.

@glyph
Copy link
Member

glyph commented Oct 13, 2016

Doesn't using branch=True have a performance impact in this case though?

No. Do you have a benchmark that suggests it does?

@wachpwnski
Copy link

@glyph I had issues where it went from 2800 requests/s to 1400 using branch, but I may have been doing it a different way. I will have to look into it in the future. Maybe it was due to chaining branches now that I think about it.

@IlyaSkriblovsky
Copy link
Contributor

I'm now choosing which approach to use to modularize my klein app. It seems to me that approach that @glyph suggested has a problem.

I'm using function like this to build reversed URLs:

def reverse_url(request, endpoint, *args, **kwargs):
    return app.url_map.bind(request.getRequestHostname()).build(endpoint, kwargs)

Using approach suggested by @glyph each sub-app will have its own router and so won't be able to build full URL to its resources.

Or may be I'm missing something and there is a better way to reverse URLs using Klein?

@glyph
Copy link
Member

glyph commented Mar 12, 2017

@IlyaSkriblovsky I don't understand your example. Where does that function live, what types does it take, how do you call it? I'm assuming this has something to do with werkzeug but I don't know what; I'm not sure what a "reversed URL" is. A runnable example would be great.

@IlyaSkriblovsky
Copy link
Contributor

Sorry for being unclear.

Let's assume we have a user's personal pages:

@app.route('/user/<username>')
def user_page(request, username):
    return 'Hi, I'm {}'.format(username)

Now we want to render an HTML page containing a link to Jonh's page. How could we generate the URL to place into href=""? Of course we could hardcode url = '/user/{}'.format('john'), but it is error-prone and not DRY. If we eventually decide to change URL format, all existing links will became invalid.

In Django this is called "reverse URL resolution" and they have a reverse() function for this. Flask has url_for() for the same purpose. Bottle has get_url().

Seems like Klein doesn't provide anything like this at the moment. But it can be easily achieved by going down to Werkzeug level:

def reverse_url(request, endpoint, *args, **kwargs):
    return app.url_map.bind(request.getRequestHostname()).build(endpoint, kwargs)

@app.route('/link-to-john')
def link_to_john(request):
    url = reverse_url(request, 'user_page', username='john')
    return '<a href="{}">John\'s page</a>'.format(url)

Klein.url_map is a Werkzeug's routing map that can build an url from endpoint name and kwargs. But now we face a problem I have started from: if each sub-app in your example has its own Klein instance, there is no way for a method inside sub-app to build the complete url to another method of the same sub-app. reverse_url() described above won't work because it will build only a part of the url under sub-app's root. It can build /add/5/7, but not /math/add/5/7. It is only possible if there is a single routing map, i.e. a single Klein instance.

Structuring apps with some kind of "Blueprints" like @tehasdf suggested in original post may not have this problem because in that case there will be only one Klein instance.

Sorry if I'm missing something because I'm new to Klein. But I think reverse resolution of URLs is a must-have feature that every web framework, even so micro-, should provide.

@IlyaSkriblovsky
Copy link
Contributor

IlyaSkriblovsky commented Mar 12, 2017

I came up with a bit different syntax for decomposition:

from klein import route, run, Klein

def users_app(app):
    @app.route("/<string:username>")
    def as_user(request, username):
        request.setHeader('content-type', 'text/plain')
        return 'A User Named "{username}".'.format(username=username)

def math_app(app):
    @app.route("/add/<int:a>/<int:b>")
    def add(request, a, b):
        return str(a + b)

app = Klein()
with app.subroute("/users"):
    users_app(app)

with app.subroute("/math"):
    math_app(app)

@app.route("/")
def hi(request):
    return "Hi"

app.run("localhost", 8123)

Why use classes if we don't actually need them? (We can't use inheritance without some tricks anyways).
If sub-apps need some external dependencies — they can be passed as args.
This way there is only one Klein instance, so we can employ werkzeug's reverse URL building as described in previous post.

@glyph
Copy link
Member

glyph commented Mar 12, 2017

Passed as args from where?

@glyph
Copy link
Member

glyph commented Mar 12, 2017

For example: I need a database connection. Who creates it and where is it passed as an arg?

@IlyaSkriblovsky
Copy link
Contributor

IlyaSkriblovsky commented Mar 12, 2017

External dependencies can be instantiated by main bootstrap code and injected into sub-apps via function arguments like this:

def users_app(app, db):
    @app.route('/<int:user_id>')
    def as_user(request, user_id):
        username = db.username_from_id(user_id)
        return 'A User Named "{username}"'.format(username=username)

db = connect_to_db()
app = Klein()
with app.subroute('/users/'):
    users_app(app, db)

app.run('localhost', 8123)

This is almost the same as passing deps to sub-app classes via __init__ args in your example. But this way we don't need to write __init__ to store them into self, we can just use them directly.

If you need sub-app-level shared state, you can have it too:

def users_app(app, db):
    username_cache = {}
    
    @app.route('/<int:user_id>')
    @defer.inlineCallbacks
    def as_user(request, user_id):
        username = username_cache.get(user_id)
        if username is None:
            username = yield db.username_from_id(user_id)
            username_cache[user_id] = username
        return 'A User Named "{}"'.format(username)

@glyph
Copy link
Member

glyph commented Mar 13, 2017

So the way to do this without a completely global db object would be something like:

def app_from_environment(environ):
    db = connect_to_db(environ['DB_DSN'])
    app = Klein()
    with app.subroute('/users/'):
        users_app(app, db)

if __name__ == '__main__':
    import os
    app = app_from_environment(os.environ)

Am I getting this right? That's... interesting. I have to admit you make an interesting case about reverse URL mapping, as this is something that I would like to be possible, and had honestly kind of given up on, since what I've seen of idiomatic web apps is that they just hard-code all the URLs.

@glyph
Copy link
Member

glyph commented Mar 13, 2017

@IlyaSkriblovsky - I think it's probably me, rather than you, who is not understanding how Klein should be in this case :). The discussion on this issue is somewhat muddled - do you want to open a fresh one explaining just your proposal?

@IlyaSkriblovsky
Copy link
Contributor

Hardcoding URLs can be ok for small apps. But if you want to create reusable sub-app (this is what this issue was originally about), you can't even hardcode them because you don't know which URL prefix you will be running at.

Thanks for discussion! I will create another issue on this topic.

@DanielAdler23
Copy link

DanielAdler23 commented Oct 18, 2017

The example above does not work if you have endpoints that have the same url and different methods(i.e - GET, POST, PUT....).
The method type always return to it's default which is GET which means that if you have two identical urls but one is used to get data from a DB(GET) and one for saving new data(POST for example) the request will be directed to the GET endpoint even though you sent a POST request

@isaacgr
Copy link

isaacgr commented Apr 4, 2020

This is already possible, and is used extensively on Mimic.

I thought that we even had an example of doing this in the documentation, but apparently you have to sort of infer it from cobbling together a combination of "return anything", "static files" and "non-global state".

The way that you do it is using branch routes, and bound apps, like so:

from klein import route, run, Klein

class UsersApp(object):
    routes = Klein()
    @routes.route("/<string:username>")
    def an_user(self, request, username):
        request.setHeader('content-type', 'text/plain')
        return 'A User Named "{username}".'.format(username=username)

class MathApp(object):
    mathroutes = Klein()
    @mathroutes.route("/add/<int:a>/<int:b>")
    def add(self, request, a, b):
        return str(a + b)


@route("/users", branch=True)
def users_app(request):
    return UsersApp().routes.resource()

@route("/math", branch=True)
def math_app(request):
    return MathApp().mathroutes.resource()

@route("/")
def hi(request):
    return "Hi"

run("localhost", 8123)

Here we have two apps - "math" and "users", each of which is "mounted" into the URL hierarchy by the top-level branch route. So you can go to /users/janet or you can go to /math/add/3/4.

This ticket is asking for an implementation, which I don't think we need, so I'm going to close it. I do think there's a valid issue here though, which is for there to be some clear documentation of this pattern, but I'm not sure how to articulate it in terms of a useful example, so I'm not opening that right away. I would definitely appreciate it if someone else could open it though!

cc @wsanchez
@glyph

I'm trying to setup something like this, but I'd like to use the root path in the subroutes.

So

class UsersApp(object):
    routes = Klein()
    @routes.route("/", methods=['GET'])
    def an_user(self, request):
        return 'This would be for getting all users'
    @routes.route("/", methods=['POST'])
    def an_user(self, request):
        return 'This would be for creating a user'

@route("/users", branch=True)
def users_app(request):
    return UsersApp().routes.resource()

However when I go to set this up I get a not found error, and only works if I do something like /getUsers which I dont want to do. Is this possible to do and if so how would I go about it?

@glyph
Copy link
Member

glyph commented Apr 5, 2020

Just off the top of my head here - that should work, have you tried adding a / to /users, ie /users/ ?

@isaacgr
Copy link

isaacgr commented Apr 5, 2020

I thought I did try that before messaging. Maybe I didnt restart the application though...Yes it appears to work if I put a / after users. Thanks for the response.

@glyph
Copy link
Member

glyph commented Apr 6, 2020

Glad we could help you out. Thanks for using Klein!

@bharath97-git
Copy link

This is already possible, and is used extensively on Mimic.
I thought that we even had an example of doing this in the documentation, but apparently you have to sort of infer it from cobbling together a combination of "return anything", "static files" and "non-global state".
The way that you do it is using branch routes, and bound apps, like so:

from klein import route, run, Klein

class UsersApp(object):
    routes = Klein()
    @routes.route("/<string:username>")
    def an_user(self, request, username):
        request.setHeader('content-type', 'text/plain')
        return 'A User Named "{username}".'.format(username=username)

class MathApp(object):
    mathroutes = Klein()
    @mathroutes.route("/add/<int:a>/<int:b>")
    def add(self, request, a, b):
        return str(a + b)


@route("/users", branch=True)
def users_app(request):
    return UsersApp().routes.resource()

@route("/math", branch=True)
def math_app(request):
    return MathApp().mathroutes.resource()

@route("/")
def hi(request):
    return "Hi"

run("localhost", 8123)

Here we have two apps - "math" and "users", each of which is "mounted" into the URL hierarchy by the top-level branch route. So you can go to /users/janet or you can go to /math/add/3/4.
This ticket is asking for an implementation, which I don't think we need, so I'm going to close it. I do think there's a valid issue here though, which is for there to be some clear documentation of this pattern, but I'm not sure how to articulate it in terms of a useful example, so I'm not opening that right away. I would definitely appreciate it if someone else could open it though!
cc @wsanchez
@glyph

I'm trying to setup something like this, but I'd like to use the root path in the subroutes.

So

class UsersApp(object):
    routes = Klein()
    @routes.route("/", methods=['GET'])
    def an_user(self, request):
        return 'This would be for getting all users'
    @routes.route("/", methods=['POST'])
    def an_user(self, request):
        return 'This would be for creating a user'

@route("/users", branch=True)
def users_app(request):
    return UsersApp().routes.resource()

However when I go to set this up I get a not found error, and only works if I do something like /getUsers which I dont want to do. Is this possible to do and if so how would I go about it?

Hey I actually tried to implement something similar to this, but even though I make a GET or a POST call, my request always goes to POST. Could you help me in this?

@glyph
Copy link
Member

glyph commented Sep 23, 2020

Hey I actually tried to implement something similar to this, but even though I make a GET or a POST call, my request always goes to POST. Could you help me in this?

Please attach a complete example: http://sscce.org

@SoundsSerious
Copy link

I see this issue is closed but its the closest topic to my concern.

The idea of reusable modules is great, as well as extendable apps or the ability to subclass apps. For example if you want to extend the base user view for different paid plans, or add a view for developers. The subclassing method i tried didn't work well, the only solution would be to merge all the underlying entities like Map ect.

@glyph
Copy link
Member

glyph commented Jun 18, 2021

@SoundsSerious Have you tried the approach suggested in #73 (comment) ?

@wsanchez
Copy link
Member

Perhaps we need to reopen this as a documentation thing.

@SoundsSerious
Copy link

@glyph @wsanchez hi! Thanks for your work over the years developing a seriously awesome framework!

I saw that approach in #73 I think that is going to be the immediate solution, my urls already fit in a complementary pattern so enough said!

Longer term i wanted to offer the idea of "adding" Klein interfaces as sets of urls (urls3 = urls1 + urls2). My initial thought process was to treat the werkzug.Map as set and union it with the child instance as a priority, but it seemed like a lot could go wrong with a cursory understanding of Klein so I thought it better to consult here. It does seem like a pretty common use case to need to join web views, so it would be awesome to have something explicitly for pluggable behavior.

@glyph
Copy link
Member

glyph commented Jun 29, 2021

Perhaps we need to reopen this as a documentation thing.

We should. This discussion is fairly discursive and meandering, so rather than reopen with this long comment history, let's continue with the documentation bit in #503

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

No branches or pull requests