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

multiple route and one function work random,404 #153

Closed
alingse opened this issue Aug 10, 2016 · 9 comments
Closed

multiple route and one function work random,404 #153

alingse opened this issue Aug 10, 2016 · 9 comments

Comments

@alingse
Copy link

alingse commented Aug 10, 2016

Here is my simple app server,use two route /api/v1/book/isbn/<str:isbn> and /api/v1/book/<int:bookid>

when I visit http://127.0.0.1:8001/api/v1/book/isbn/1231
It works 50% work at #return 1 ,50% 404,
I dont know why,It has run inside the book function already。

This is my code:

#coding=utf-8
from weppy import request
from weppy import App
from weppy.tools import service

app = App(__name__)

import random

@app.route('/api/v1/book/isbn/<str:isbn>')
@app.route('/api/v1/book/<int:bookid>')
@service.json
def book(isbn=None,bookid=None):

    if random.random()>0.5:
    #return 1
        return isbn
    #return 2
    if isbn != None:
        data = {'isbn':isbn}
    if bookid != None:
        bookid = int(bookid)
        data = {'id':bookid}

    return data

if __name__ == '__main__':
    app.debug = True
    app.run(host='0.0.0.0', port=8001)
@gi0baro
Copy link
Member

gi0baro commented Aug 11, 2016

Hi @alingse, I'm sorry but right now multiple route decorations are not supported.
This is already on issues (see #108) but have no ETA.

The current weppy wants you to write something like this (is ugly, I know):

def book(isbn=None,bookid=None):
    # code returning data

@app.route('/api/v1/book/isbn/<str:isbn>')
@service.json
def book_wisbn(isbn):
    return book(isbn=isbn)

@app.route('/api/v1/book/<int:bookid>')
@service.json
def book_wid(bookid):
    return book(bookid=bookid)

@hydeparkk
Copy link
Contributor

Hi @gi0baro, I've done little research and it looks like multiple route decorations works fine, but the problem here is with @service.json decorator.
I've created pull request (#155) with changes to make this work properly.

@alingse
Copy link
Author

alingse commented Aug 11, 2016

Hi @gi0baro , I have just test the package post by @hydeparkk ,his weppy works well.

and I do some test on latest weppy(0.7.8)

....
@app.route('/api/v1/book/isbn/<str:isbn>')
@app.route('/api/v1/book/<int:bookid>')
#@service.json
def book(isbn=None,bookid=None):
    if True:
        return str([bookid,isbn])
...

It also works well,
http://127.0.0.1:8001/api/v1/book/isbn/1231 --> [None, '1231']
http://127.0.0.1:8001/api/v1/book/123123 --> ['123123', None]

I try say more thing about my first code

    if random.random()>0.5:
    #return 1
        return isbn
    #return 2
    ....
    return data

just go into return 1 is always works,but when go into return 2,It became 404.

I think something wrong while the code going to @service.json to visit and return the args isbnor idin function book.

@gi0baro
Copy link
Member

gi0baro commented Aug 11, 2016

@alingse please use the code I given to test your code.

It probably goes to 404 because it's searching for a template since you don't have the json service anymore.

@alingse
Copy link
Author

alingse commented Aug 12, 2016

Yes, @gi0baro ,
I test the code you given,
It works well, whatever it returns (list or dict or string).

And Inspired by your code and the comment
I find it might need twice @service.json for every route.

test 1.
It works too, and return right on list and dict

@app.route('/api/v1/book/isbn/<str:isbn>')
@service.json
@app.route('/api/v1/book/<int:bookid>')
@service.json
def book(isbn=None,bookid=None):
    return [isbn,bookid]

http://127.0.0.1:8001/api/v1/book/isbn/1231 --> ["1231", null]
http://127.0.0.1:8001/api/v1/book/1231 --> [null, "1231"]

test 2.
It is simplified from my first post

@app.route('/api/v1/book/isbn/<str:isbn>')
@app.route('/api/v1/book/<int:bookid>')
@service.json
def book(isbn=None,bookid=None):
    return {'isbn':isbn,'bookid':bookid}

http://127.0.0.1:8001/api/v1/book/isbn/1231 --> 404
http://127.0.0.1:8001/api/v1/book/1231 --> {"isbn": null, "bookid": "1231"}

test 3.
It works but not right .

@app.route('/api/v1/book/isbn/<str:isbn>')
@app.route('/api/v1/book/<int:bookid>')
@service.json
def book(isbn=None,bookid=None):
    return [isbn,bookid,isbn,bookid]

http://127.0.0.1:8001/api/v1/book/isbn/1231 --> 12311231
http://127.0.0.1:8001/api/v1/book/123123 --> [null, "123123", null, "123123"]
the bookid None goto empty.
It seems the response use the default serializers for a list without service.json

the python‘s decorator order or different serializers might was the reason
(while return dict, it goto 404)?
but I still know nothing.

I choose to use twice @service.json

Thank you && Thank @hydeparkk too.

@gi0baro
Copy link
Member

gi0baro commented Aug 12, 2016

@alingse thank you for sharing your experiments. Will keep this open until complete support for multiple routes is implemented so I can update you about the status. There's no official ETA, but probably will be included in weppy 1.0, scheduled for the end of the year/start of the next year.

@gi0baro
Copy link
Member

gi0baro commented Dec 19, 2016

@alingse I have an update on this.
Since 6d1845c (master pre 1.0) the route decorator allow to use multiple paths.
Your example should now be written:

@app.route(['/api/v1/book/isbn/<str:isbn>', '/api/v1/book/<int:bookid>'])
@service.json
def book(isbn=None,bookid=None):
    # code

When you need to build reverse routing with the url helper, the first route is always applied:

>>> url('book', 'fe23')
/api/v1/book/isbn/fe23

if you need anyone of the other paths, you can use an underscore notation with the position in the array:

>>> url('book_1', 123)
/api/v1/book/123

@alingse I would love to hear your feedback on this. If you are satisfied with the solution, I'm gonna close this and update the docs for the 1.0

@alingse
Copy link
Author

alingse commented Dec 19, 2016

nice , this can be closed

Route class is much better

and @route accepts multi paths is also interesting.

yet I have two advice about url in issues 173

@gi0baro
Copy link
Member

gi0baro commented Dec 19, 2016

@alingse ok, I'm closing this.

@gi0baro gi0baro closed this as completed Dec 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

No branches or pull requests

3 participants