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

the behavior of the trailing slash should be more intuitive and consistent #47

Closed
AlJohri opened this Issue Jun 7, 2017 · 7 comments

Comments

Projects
None yet
2 participants
@AlJohri

AlJohri commented Jun 7, 2017

I want a trailing slash for routes with parameters, such as /api/v1/templates/1/. any way to achieve that? it gives a 404 right now and only works without the trailing slash

@hoatle

This comment has been minimized.

Show comment
Hide comment
@hoatle

hoatle Jun 7, 2017

Member

there is a note on docs:

"
Note
Flask-Classful favors putting trailing slashes at the end of routes without parameters. You can override this behavior by specifying trailing_slash=False either as an attribute of your FlaskView or in the register method.
"

from: http://flask-classful.teracy.org/#flask-classful-s-way-of-talking-about-routes

by default, a trailing slash should work. Could you provide the sample code to see if there is anything wrong?

Member

hoatle commented Jun 7, 2017

there is a note on docs:

"
Note
Flask-Classful favors putting trailing slashes at the end of routes without parameters. You can override this behavior by specifying trailing_slash=False either as an attribute of your FlaskView or in the register method.
"

from: http://flask-classful.teracy.org/#flask-classful-s-way-of-talking-about-routes

by default, a trailing slash should work. Could you provide the sample code to see if there is anything wrong?

@hoatle

This comment has been minimized.

Show comment
Hide comment
@hoatle

hoatle Jun 7, 2017

Member

related: #46 for the next release version that trailing slash can be optional.

Member

hoatle commented Jun 7, 2017

related: #46 for the next release version that trailing slash can be optional.

@AlJohri

This comment has been minimized.

Show comment
Hide comment
@AlJohri

AlJohri Jun 7, 2017

@hoatle here is an example app: https://github.com/AlJohri/nonworking-flask-apispec-example

I was using it to debug a flask-apispec issue jmcarp/flask-apispec#54 (comment) but you can see the trailing slash issue here as well.

The routes that work are:

The route that doesn't work is:

AlJohri commented Jun 7, 2017

@hoatle here is an example app: https://github.com/AlJohri/nonworking-flask-apispec-example

I was using it to debug a flask-apispec issue jmcarp/flask-apispec#54 (comment) but you can see the trailing slash issue here as well.

The routes that work are:

The route that doesn't work is:

@AlJohri

This comment has been minimized.

Show comment
Hide comment
@AlJohri

AlJohri Jun 7, 2017

The relevant code is:

class TemplateResource(FlaskView, metaclass=ResourceMeta):
    route_base = '/templates/'
    trailing_slash = True

    @marshal_with(TemplateSchema(many=True))
    def index(self):
        return Template.query.all()

    @marshal_with(TemplateSchema)
    def get(self, template_id):
        template = Template.query.filter(Template.id == template_id).one()
        return template

TemplateResource.register(app)

The issue occurs with trailing_slash set to True or False.

AlJohri commented Jun 7, 2017

The relevant code is:

class TemplateResource(FlaskView, metaclass=ResourceMeta):
    route_base = '/templates/'
    trailing_slash = True

    @marshal_with(TemplateSchema(many=True))
    def index(self):
        return Template.query.all()

    @marshal_with(TemplateSchema)
    def get(self, template_id):
        template = Template.query.filter(Template.id == template_id).one()
        return template

TemplateResource.register(app)

The issue occurs with trailing_slash set to True or False.

@hoatle

This comment has been minimized.

Show comment
Hide comment
@hoatle

hoatle Jun 8, 2017

Member

hello @AlJohri, this works by using route

from flask import Flask
from flask_classful import FlaskView, route

# we'll make a list to hold some quotes for our app
quotes = [
    "A noble spirit embiggens the smallest man! ~ Jebediah Springfield",
    "If there is a way to do it better... find it. ~ Thomas Edison",
    "No one knows what he can do till he tries. ~ Publilius Syrus"
]

app = Flask(__name__)

class QuotesView(FlaskView):
    def index(self):
        return "<br>".join(quotes)

    @route('/<id>/')
    def get(self, id):
      id = int(id)
      if id < len(quotes) - 1:
          return quotes[id]
      else:
          return "Not Found", 404

By using route as above, /quotes/<id> is redirected to /quotes/<id>/ and it should work.

By default (without route shown above, the current behavior is /quotes/<id> works only).

Could you try with route to see if it works?

Member

hoatle commented Jun 8, 2017

hello @AlJohri, this works by using route

from flask import Flask
from flask_classful import FlaskView, route

# we'll make a list to hold some quotes for our app
quotes = [
    "A noble spirit embiggens the smallest man! ~ Jebediah Springfield",
    "If there is a way to do it better... find it. ~ Thomas Edison",
    "No one knows what he can do till he tries. ~ Publilius Syrus"
]

app = Flask(__name__)

class QuotesView(FlaskView):
    def index(self):
        return "<br>".join(quotes)

    @route('/<id>/')
    def get(self, id):
      id = int(id)
      if id < len(quotes) - 1:
          return quotes[id]
      else:
          return "Not Found", 404

By using route as above, /quotes/<id> is redirected to /quotes/<id>/ and it should work.

By default (without route shown above, the current behavior is /quotes/<id> works only).

Could you try with route to see if it works?

@hoatle

This comment has been minimized.

Show comment
Hide comment
@hoatle

hoatle Jun 8, 2017

Member

I guess we should check back the behavior of the trailing slash to get it more intuitive for the next version.

Member

hoatle commented Jun 8, 2017

I guess we should check back the behavior of the trailing slash to get it more intuitive for the next version.

@hoatle hoatle changed the title from trailing slash for routes with parameters? to the behavior of the trailing slash should be more intuitive and consistent Sep 9, 2017

hoatle added a commit to hoatle/flask-classful that referenced this issue Sep 9, 2017

hoatle added a commit to hoatle/flask-classful that referenced this issue Sep 9, 2017

hoatle added a commit to hoatle/flask-classful that referenced this issue Sep 9, 2017

hoatle added a commit to hoatle/flask-classful that referenced this issue Sep 9, 2017

hoatle added a commit that referenced this issue Sep 9, 2017

Merge pull request #55 from hoatle/bugs/#47-trailing-slash
@ #47 | the behavior of the trailing slash should be more intuitive and consistent
@hoatle

This comment has been minimized.

Show comment
Hide comment
@hoatle

hoatle Sep 10, 2017

Member

hello @AlJohri, it's considered as a bug from your report and it should be fixed now.

Member

hoatle commented Sep 10, 2017

hello @AlJohri, it's considered as a bug from your report and it should be fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment