hfaran / Tornado-JSON

A simple JSON API framework based on Tornado
http://tornado-json.readthedocs.org/
MIT License
273 stars 60 forks source link

Proposal for a new Routing system. #45

Open Deiru2k opened 10 years ago

Deiru2k commented 10 years ago

I have been trying out the automatic routing system that you currently use in project, and I have found it to be not as convenient as I think it should be. While it certainly is suited for some cases (i.e. when you store all your views inside one file, say, api.py), I found out that it doesn't work well with the project layout I find most convenient (and I have good reasons for that). Currently, it walks a package for modules and creates routes respectively, so handlers.api.ImagesHandler becomes /api/images But when working with larger projects, I find this to be not as convenient as it might seem. The way I use my project, is that I create a handlers package and create modules for each resource I use respectively, so in my case with your current auto-routing, the module will be handlers.images.ImagesHandler and handlers.images.ImageHandler Which would be mapped to /images/images and /images/image. What I propose is a system that will A - have a defined base path for routes that can be defined when calling get_routes (i.e. get_routes(package, base='api')) And names will be generated using the following scheme handlers.images.ImagesHandler -> /api/images handlers.images.ImageHandler -> /api/images/(.* ) With any additional loose handlers that don't contain module name within them will become actions for collection, like handlers.images.PopularHandler -> /api/images/popular With exceptions and custom routes that can be defined inside Handler classes.

This system is more suited for bigger projects where having a separate module for each resource can be pretty convenient and makes it easier to navigate the project.

I understand that you might be focused on other things, or don't really feel like implementing such system, so I've decided to take it up on my own and fill a pull-request once I'm done. I would also try to implement changes I've proposed in my previous issue regarding metadata in jsend.

hfaran commented 10 years ago

Hey there!

I see where you're coming from on this; the basic routing system that exists by default in Tornado-JSON is very basic as I've noted in the documentation. That was my reasoning for adding on URL Annotations. I believe what you're trying to accomplish could either be done using URL Annotations or fundamentally changing the routing system. Both are valid options.

Either way, I'm definitely open to extensions on the routing system to see if it can be made better. I am a little busy with other things at the moment so I do not have time to implement this myself, however, a pull request from you on this would be most welcome! :smiley:

Deiru2k commented 10 years ago

If you have to use annotations to achieve this goal, doesn't routing system stop being automatic? Annotations are helpful in their own way, but it's kinda pointless to use the system if you have to annotate every handler you make. I will try to implement the system in my own vision without changing the current one, adding a new option instead, and file a pull-request.

hfaran commented 10 years ago

Well, what I was getting at with the annotations was that you could automate the usage of those somehow, but yes, that would indeed be circumventing the built-in routing automation in favour of a hand-rolled one.

But regardless, if you can make your vision happen as an extension of the current routing system, then fantastic! I await your pull request! :smiley:

kanoc commented 9 years ago

A more flexible routing system would be really nice, as the current implementation is severely limited. In a test project I ended up setting up the routes manually.

Besides the module names issue (which could be solved by Misaka42's proposal), it would be nice to have more options when generating routes with parameters that get passed to the HTTP methods. For example, imagine we have a resource (ID 123) that has a list of items and we want to have a Handler that gets one item (ID 456) and we need both the resource and item IDs. With current implementation the URL looks like /resource/123/456. It would be nice to have an URL /resource/123/item/456 instead.

hfaran commented 9 years ago

Thanks for your suggestions! I completely agree with you on the limitations; I believe I have some time over the next month to look into how routing can be made more flexible and implementing your suggestions.

It seems like @Misaka42 got started on an improved implementation but most likely got busy with other things.

auvipy commented 9 years ago

how about werkzeug like routing?

auvipy commented 9 years ago

http://truemped.github.io/tornadotools/route.html

auvipy commented 9 years ago

http://tornado-smack.readthedocs.org/en/latest/

hfaran commented 9 years ago

@auvipy Yes! This is along the lines of what I had in mind: https://github.com/hfaran/Tornado-JSON/compare/routing_refactor#diff-282bf316c179404e83b2e5db1ce3170bR256

I have not unfortunately had time to touch this in a while though because I've been quite busy recently but I hope to flesh this out more and finish it soon.

hfaran commented 9 years ago

https://github.com/troolee/tornado-routes