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

Replacement markers in URL dispatcher raise pyramid.exceptions.ConfigurationExecutionError on arbitrary regex #629

Closed
avanov opened this Issue Jul 2, 2012 · 4 comments

Comments

Projects
None yet
2 participants
@avanov
Contributor

avanov commented Jul 2, 2012

Documentation says that

...the replacement marker {foo} can more verbosely be spelled as {foo:[^/]+}. You can change this to be an arbitrary regular expression to match an arbitrary sequence of characters, such as ...

However, the markers cannot handle any regex which contains colons (for example, non-capturing regular parentheses (?:))

  File "/home/ghostwriter/virtualenv/myapp/local/lib/python2.7/site-packages/pyramid/config/__init__.py", line 921, in make_wsgi_app
    self.commit()
  File "/home/ghostwriter/virtualenv/myapp/local/lib/python2.7/site-packages/pyramid/config/__init__.py", line 594, in commit
    self.action_state.execute_actions(introspector=self.introspector)
  File "/home/ghostwriter/virtualenv/myapp/local/lib/python2.7/site-packages/pyramid/config/__init__.py", line 1049, in execute_actions
    tb)
  File "/home/ghostwriter/virtualenv/myapp/local/lib/python2.7/site-packages/pyramid/config/__init__.py", line 1041, in execute_actions
    callable(*args, **kw)
  File "/home/ghostwriter/virtualenv/myapp/local/lib/python2.7/site-packages/pyramid/config/routes.py", line 422, in register_connect
    pregenerator=pregenerator, static=static
  File "/home/ghostwriter/virtualenv/myapp/local/lib/python2.7/site-packages/pyramid/urldispatch.py", line 62, in connect
    route = Route(name, pattern, factory, predicates, pregenerator)
  File "/home/ghostwriter/virtualenv/myapp/local/lib/python2.7/site-packages/pyramid/urldispatch.py", line 35, in __init__
    self.match, self.generate = _compile_route(pattern)
  File "/home/ghostwriter/virtualenv/myapp/local/lib/python2.7/site-packages/pyramid/urldispatch.py", line 151, in _compile_route
    name, reg = name.split(':')
pyramid.exceptions.ConfigurationExecutionError: <type 'exceptions.ValueError'>: too many values to unpack

This issue may be resolved either by providing the maxsplit=1 argument to name.split(':') statement, or by introducing requirements argument to config.add_route().

Note that the Routes package supports both approaches and it makes a huge difference in some situations. For example, consider the following case:

We have the following URL pattern - /{game_tagname}/dlc/{tagname} in which game_tagname and tagname have the same regex pattern signature. With the current implementation of inlined regular expressions I have to write something like this:

TAGNAME_RE = 'some_regex'
config.add_route(
    'name',
    '/{game_tagname:' + TAGNAME_RE + '}/dlc/{tagname:' + TAGNAME_RE '}'
)

whereas Routes allows me to write this

TAGNAME_RE = 'some_regex'
map.connect(
    'name',
    '/{game_tagname}/dlc/{tagname}',
    requirements=dict(game_tagname=TAGNAME_RE, tagname=TAGNAME_RE)

The problem is that a big system can have a number of reusable regular route patterns, and Routes' requirements argument is actually a big win. It allows me to combine regular expressions with each other to compose predifined dictionaries of rules and bind them to unique names (as constants).

@mmerickel

This comment has been minimized.

Show comment
Hide comment
@mmerickel

mmerickel Jul 2, 2012

Member
def name_reqs(info, request):
    match = info['match']
    return all(TAGNAME_RE.match(match[arg]) for arg in ('game_tagname', 'tagname'))

config.add_route('name', '/{game_tagname}/dlc/{tagname}', custom_predicates=[name_reqs])

The routes package is insanely complicated (just ask its author). Pyramid's regexes are simple but it provides a very powerful, albeit slightly more verbose, mechanism (predicates) for getting the behavior you desire.

Member

mmerickel commented Jul 2, 2012

def name_reqs(info, request):
    match = info['match']
    return all(TAGNAME_RE.match(match[arg]) for arg in ('game_tagname', 'tagname'))

config.add_route('name', '/{game_tagname}/dlc/{tagname}', custom_predicates=[name_reqs])

The routes package is insanely complicated (just ask its author). Pyramid's regexes are simple but it provides a very powerful, albeit slightly more verbose, mechanism (predicates) for getting the behavior you desire.

@avanov

This comment has been minimized.

Show comment
Hide comment
@avanov

avanov Jul 2, 2012

Contributor

Will the route with such name_reqs predicate perform two match operations (default [^/]+, and that is specified in name_reqs) for each marker?

Contributor

avanov commented Jul 2, 2012

Will the route with such name_reqs predicate perform two match operations (default [^/]+, and that is specified in name_reqs) for each marker?

@mmerickel

This comment has been minimized.

Show comment
Hide comment
@mmerickel

mmerickel Jul 2, 2012

Member

I'm not sure I understand. If the route regex matches then the predicates are matched. If everything passes then route lookup is complete. If anything fails it continues to the next route. Whatever you specify as the route regex is what will be required to be matched before the predicates are invoked.

Member

mmerickel commented Jul 2, 2012

I'm not sure I understand. If the route regex matches then the predicates are matched. If everything passes then route lookup is complete. If anything fails it continues to the next route. Whatever you specify as the route regex is what will be required to be matched before the predicates are invoked.

@mmerickel

This comment has been minimized.

Show comment
Hide comment
@mmerickel

mmerickel Jul 10, 2012

Member

Fixed in #634

Member

mmerickel commented Jul 10, 2012

Fixed in #634

@mmerickel mmerickel closed this Jul 10, 2012

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