Handles Pyramid path regex and subpaths #70
Conversation
Support path regex and subpath matching in Pyramid routes. Subpaths are considered regular path attributes.
There was a problem hiding this comment.
Thanks for taking the time to read through all the Pyramid documentation. My head started to spin by the time I got to "There are only four real differences between a purely traversal-based application and a hybrid application".
I think the approach taken here is a good one. Maybe I didn't understand the question but I don't think we can really offer much for *traverse and *subpath without a lot of complexity in doing traversals ourselves. I'd prefer to leave this alone until we get a feature request with a concrete use case.
I'm not an expert on Pyramid by any means so feel free to correct me if my understanding of *subpath and *traverse in my review comments is wrong :)
| # handle traverse and subpath | ||
| # docs.pylonsproject.org/projects/pyramid/en/latest/narr/hybrid.html | ||
| for subpath_marker in ('*subpath', '*traverse'): | ||
| if name.find(subpath_marker) > 0: |
There was a problem hiding this comment.
Should this be >= 0? Or should we just use subpath_marker in name?
| # docs.pylonsproject.org/projects/pyramid/en/latest/narr/hybrid.html | ||
| for subpath_marker in ('*subpath', '*traverse'): | ||
| if name.find(subpath_marker) > 0: | ||
| name = subpath_marker[1:] |
There was a problem hiding this comment.
Is it possible to have a pattern for *subpath or *traversal? Should we just return here to make it obvious that the pattern part of this function isn't relevant, or maybe move this stanza to the top of the function?
There was a problem hiding this comment.
I'm not sure about this one, but I think we can just ignore the pattern in such cases.
| @@ -145,8 +144,10 @@ def from_path(self, path): | |||
| :type path: string | |||
| :rtype: list | |||
| """ | |||
| path_components = path.split('/') | |||
| param_names = [comp[1:-1] for comp in path_components | |||
| if comp.startswith('{') and comp.endswith('}')] | |||
There was a problem hiding this comment.
The documentation at http://docs.pylonsproject.org/projects/pyramid/en/latest/narr/hybrid.html makes it seem like *traverse and *subpath don't occur in {/} but just show up at the end of the path.
tests/test_parameter_handler.py
Outdated
| params = self.handler.from_path('/my/{param:\\d+}/path/{id:[a-z]{8,42}}') | ||
| names = [param['name'] for param in params] | ||
| expected = ['param', 'id'] | ||
| self.assertEqual(sorted(names), sorted(expected)) |
There was a problem hiding this comment.
Nice; maybe we should add a test to verify that the patterns get captured too?
cornice_swagger/swagger.py
Outdated
| @@ -148,6 +148,12 @@ def from_path(self, path): | |||
| param_names = [comp[1:-1] for comp in path_components | |||
| if comp.startswith('{') and comp.endswith('}')] | |||
|
|
|||
| # handle traverse and subpath | |||
| # docs.pylonsproject.org/projects/pyramid/en/latest/narr/hybrid.html | |||
| for subpath_marker in ('*subpath', '*traverse'): | |||
There was a problem hiding this comment.
We also need to update the swagger path in here.
I agree, by this approach I just want to make sure that the result is valid. and that we do include a way to provide the subpath on the documentation. |
| # handle traverse and subpath as regular parameters | ||
| # docs.pylonsproject.org/projects/pyramid/en/latest/narr/hybrid.html | ||
| for subpath_marker in ('*subpath', '*traverse'): | ||
| path = path.replace(subpath_marker, '{subpath}') |
There was a problem hiding this comment.
If we assume that subpath and traverse are always outside { .* } we can do something like this.
Fixes: #68
Related to: Kinto/kinto#1180
Support path regex and subpath matching in Pyramid routes.
Considerations:
Notes:
r? @glasserc