bottle icon indicating copy to clipboard operation
bottle copied to clipboard

Proposal for request global removal

Open foxx opened this issue 9 years ago • 12 comments

Currently the request context is stored in the request global, which can encourage anti patterns such as accessing the request object from a database model.

I'm thinking that a cleaner approach would be to pass in the request context to the view function itself, rather than having to access it from a global. This object would then be passed around as/where it was needed, encouraging modularity by design.

def some_view(request):
    print request

Could a core maintainer give their thoughts?

foxx avatar Jun 05 '15 16:06 foxx

Yes, you are absolutely right. Problem is: Changing this aspect would break any application ever. The only feasible path I can think of:

0.14. Add a plugin to the core that implements this (perhaps based on bottle-inject?). 0.15. Make the plugin the default and allow the user to explicitly disable it. 0.16. Make the plugin core functionality and instead add a plugin that mimics the old behavior.

The debian guys will probably patch the default to stay the same though, because Bottle 0.x is already used a lot in production. A real API change can only be done with a new major release, but as long as there is a migration path, things should be feasible.

If (and I say if) we want to change this core aspect of the framework, we should do it before 1.0. Would be a nice variation from the almost identically (since copied) Flask API, too.

defnull avatar Jun 05 '15 20:06 defnull

LGTM, there certainly needs to be a clear migration path, and plugins seem like the best fit. If this gets accepted then I'll happily contribute some time for a PR.

foxx avatar Jun 05 '15 21:06 foxx

Personally, I like the way django does it. The idea of using an internal plugin is quite nice and should be trivial to implement.

@my_app.route('/')
def home_page(request):
    return 'hello, world!'

The syntax seems quite nice, and already fits in with the existing plugin architecture. I'll see if I can write up a simple PR.

I propose 0.13 has a way to expose the new functionality (albeit disabled by default) and 0.14 enables the the plugin functionality by default. There can even be ways to keep the old API around if users really need it.

Moreover, we'll need to include disclaimers in the docs about the changes. This API change will undoubtedly cause confusion to newcomers.

camconn avatar Jun 13 '15 02:06 camconn

Implementing such a feature (again, the bottle-inject plugin already is capable of doing this) should be the easy part. The documentation part is probably the hardest one. I suggest we start a new branch for this.

defnull avatar Jun 14 '15 16:06 defnull

Not a huge fan of this change. request is a nice way to pass session state (ie current user) into templates, without having to forward it at every call to include.

eric-wieser avatar Jun 30 '15 12:06 eric-wieser

@eric-wieser I'm not sure I understand what your issue is. You'd just pass in the request object as normal into your templating library. Unless you're saying that you prefer your templating library to magically detect request from globals, rather than having to pass it in on render(), which is not a recommended approach and is precisely what this proposed change is trying to prevent. If I have misunderstood please let me know

foxx avatar Jul 06 '15 13:07 foxx

I'm trying to cover this situation, using STPL:

main.tpl

include('components/user-image.tpl')

components/user-image

% if request.user:
    <img src="...">
% else:
    <p>For privacy reasons, images are not shown for anonymous users</p>
% end

I don't want to have to forward the request argument at every step

eric-wieser avatar Jul 06 '15 16:07 eric-wieser

I changed the default a while ago to automatically forward all local variables to the included template. So, passing request or is_user=? to main.tpl should be enough.

https://github.com/bottlepy/bottle/blob/727ce6d7972133997d8ef5ec3ce290646796d4c9/bottle.py#L3524

defnull avatar Aug 17 '15 19:08 defnull

@defnull Are you still happy to have this included in 0.13? Any change of plan (re. plugins) from the previous comments?

foxx avatar Aug 17 '15 22:08 foxx

@defnull Any thoughts on whether this will make it into the core?

foxx avatar Jan 22 '16 19:01 foxx

If you implement this feature, could you solve this other question? https://github.com/bottlepy/bottle/issues/906

agalera avatar May 17 '17 20:05 agalera

If you do this for request then please also do this for response.

rudolphfroger avatar Jan 13 '20 07:01 rudolphfroger