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

Routes gets generated differently with route_prefix #2143

Closed
tholo opened this issue Nov 19, 2015 · 4 comments
Closed

Routes gets generated differently with route_prefix #2143

tholo opened this issue Nov 19, 2015 · 4 comments

Comments

@tholo
Copy link

tholo commented Nov 19, 2015

So I encountered an issue when trying to use pyramid_swagger for a project, where it would sometimes not match my requests to the spec.

I tracked this down to different behavior depending on how the routes were created, in that any path that was fully qualified from the outset generates a fully qualified path from root (a leading "/"), while those generated by utilizing config.include(..., route_prefix='/some/prefix') generates entries without the leading "/".

The following simple code illustrates the difference:

from pyramid.config import Configurator


def main(global_config, **settings):
    """This function returns a Pyramid WSGI application."""
    config = Configurator(settings=settings)
    config.add_route('home', '/some/path')
    config.include(__name__, route_prefix='/api/v1')
    return config.make_wsgi_app()


def includeme(config):
    config.add_route('users', '/users')


if __name__ == '__main__':
    app = main({}, settings={})

    print(app.routes_mapper.routes['home'].path)
    print(app.routes_mapper.routes['users'].path)

And the corresponding output from that looks like:

/some/path
api/v1/users

Note how one of the entries has the leading "/" while the other does not...

@mmerickel
Copy link
Member

In pyramid the leading / is irrelevant. Also the route mapper is actually a private API. What's pyramid_swagger attempting to do that it cannot with Pyramid's public APIs?

@tholo
Copy link
Author

tholo commented Feb 3, 2016

It is apparently trying to validate the request against the Swagger spec (that it is one of the documented requests and that it further has all required information and such).

Note that at the time this is being done, "request.matched_route" is None. Also the "pattern" member of the route object (which I assume is what should have been set in matched_route — perhaps this tween is too early for that to happen?) is also set without the leading "/".

And yes, I do realize that Pyramid does not differentiate between a "path/to/request" vs. a "/path/to/request", but I would kind of hope it would at least be consistent in which ones it generates (using route_prefix with config.include vs. not); the config.include(route_prefix='/some/path') did have the prefix, yet the generated one did not.

Note that I am just (trying to be) a consumer of both of these packages. 😄

@mmerickel
Copy link
Member

@tholo if you're interested in working on this it should be a simple fix. I opened #2407 to track it for you. It's not critical though so if you aren't interested it may not get fixed anytime soon.

@tholo
Copy link
Author

tholo commented Oct 7, 2016

This was fixed downstream in Yelp/pyramid_swagger/pull/151

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

2 participants