Make proutes more readable and report more info. #1431

sontek · 2014-10-17T03:39:22Z

Currently proutes doesn't report what request_method's are accepted
and the print out has static padding defined so it becomes unreadable
very quickly.

Prior to this change proutes looked like this:

Name            Pattern                        View                     
----            -------                        ----                     
___debug_toolbar/static/ /_debug_toolbar/static/*subpath <function <pyramid.static.static_view object at 0x7f75698a3190> at 0x7f7566c688c0>
debugtoolbar.source /_debug_toolbar/source         <function ExceptionDebugView at 0x7f75659abb90>
debugtoolbar.execute /_debug_toolbar/execute        <function ExceptionDebugView at 0x7f75659abcf8>
debugtoolbar.console /_debug_toolbar/console        <function ExceptionDebugView at 0x7f75659abe60>
debugtoolbar.exception /_debug_toolbar/exception      <function ExceptionDebugView at 0x7f75659aba28>
debugtoolbar.sql_select /_debug_toolbar/sqlalchemy/sql_select <function SQLAlchemyViews at 0x7f75659b7050>
debugtoolbar.sql_explain /_debug_toolbar/sqlalchemy/sql_explain <function SQLAlchemyViews at 0x7f75659b71b8>
response-rollups /v1/responses/rollup           <pyramid.config.views.MultiView object at 0x7f75659b6750>
respondents     /v1/respondents                <function get_respondents at 0x7f75659b7668>

With this change it looks like this:

Name                          Pattern                                     View                                                                      Method     
----                          -------                                     ----                                                                      ------     
___debug_toolbar/static/      /_debug_toolbar/static/*subpath             pyramid.static.<pyramid.static.static_view object at 0x7f36d1f0fc10>      ALL        
debugtoolbar.source           /_debug_toolbar/source                      pyramid_debugtoolbar.views.ExceptionDebugView                             ALL        
debugtoolbar.execute          /_debug_toolbar/execute                     pyramid_debugtoolbar.views.ExceptionDebugView                             ALL        
debugtoolbar.console          /_debug_toolbar/console                     pyramid_debugtoolbar.views.ExceptionDebugView                             ALL        
debugtoolbar.exception        /_debug_toolbar/exception                   pyramid_debugtoolbar.views.ExceptionDebugView                             ALL        
debugtoolbar.sql_select       /_debug_toolbar/sqlalchemy/sql_select       pyramid_debugtoolbar.views.SQLAlchemyViews                                ALL        
debugtoolbar.sql_explain      /_debug_toolbar/sqlalchemy/sql_explain      pyramid_debugtoolbar.views.SQLAlchemyViews                                ALL        
response-rollups              /v1/responses/rollup                        sonteksvc.v1.views.responses.get_response_rollup                          POST,GET   
respondents                   /v1/respondents                             sonteksvc.v1.views.responses.get_respondents                              POST

Currently proutes doesn't report what request_method's are accepted and the print out has static padding defined so it becomes unreadable very quickly.

func_name was deprecated in Python 3

aconrad · 2014-10-17T19:38:23Z

Very pretty!

stevepiercy · 2014-10-17T20:03:31Z

I like how this PR sets the column width according to the maximum length of an item in a column plus 5 spaces of padding, assuming I interpret it correctly. Nicely done!

I'm still tentative about merging PRs with code, though, so I'll leave the merging to a senior dev.

mmerickel · 2014-10-17T20:12:06Z

Is the request method so important? I've never written an app that used the request_method predicate on a route and it may be deceiving here when debugging to see ALL and not understand why you're getting a 404. Just wanted to bring this up, but the output is definitely much improved.

sontek · 2014-10-18T07:49:46Z

@mmerickel I think its useful, we pretty much always use request_method on the route because you can have a RESTful API using PUT, POST, PATCH but utilize the same view for a lot of it

witsch · 2014-10-18T15:05:33Z

On 18 Oct 2014, at 09:49, John Anderson [email protected] wrote:

@mmerickel I think its useful, we pretty much always use request_method on the route because you can have a RESTful API using PUT, POST, PATCH but utilize the same view for a lot of it

+1 (and yes, this looks much better! :))=

mmerickel · 2014-10-18T16:00:26Z

@sontek Oh? You don't use view predicates for that?

sontek · 2014-10-18T17:42:21Z

@mmerickel Yeah, we have a mix of both ways. This PR is currently only pulling the request method from the view the routes are attached to, but it might make sense to pull from the route as well. For example, you could cause a conflict like this:

from pyramid.view import view_config
from pyramid.response import Response


@view_config(route_name='home', request_method='POST')
@view_config(route_name='home', request_method='PUT')
def home(request):
    return Response("Hello Moon!")

from pyramid.config import Configurator


def main(global_config, **settings):
    config = Configurator(settings=settings)
    config.add_route('home', '/', request_method='PATCH')

    config.scan()
    return config.make_wsgi_app()

sontek · 2014-10-18T19:44:28Z

@mmerickel The latest commit makes it prefer the route request method over the view request method. That way it wont return a request method that isn't actually available. This makes a project like this work:

from pyramid.view import view_config
from pyramid.response import Response


@view_config(route_name='home', request_method='POST')
@view_config(route_name='home', request_method='PUT')
@view_config(route_name='home', request_method='PATCH')
def home(request):
    return Response("Hello Moon!")

@view_config(route_name='vacation', request_method='PUT')
def vacation(request):
    return Response("Hello Moon!")



from pyramid.config import Configurator


def main(global_config, **settings):
    config = Configurator(settings=settings)
    config.add_route('home', '/', request_method='PATCH')
    config.add_route('vacation', '/vacation')

    config.scan()
    return config.make_wsgi_app()

This would return:

Name          Pattern        View                              Method     
----          -------        ----                              ------     
home          /              pyramid_hello.views.home          PATCH      
vacation      /vacation      pyramid_hello.views.vacation      PUT

aconrad · 2014-10-18T19:51:15Z

@sontek I'd use the wording ANY rather than ALL. What do you think?

sontek · 2014-11-06T21:16:12Z

@mmerickel This is like the most important 1.6 feature of all time.

mmerickel · 2014-11-10T06:55:30Z

I'm fine with merging this as-is but under duress. Even in this system of guessing the request method from the route and then the view, there are still plenty of scenarios that this will improperly display a method for which I feel is deceiving to a new user of this utility. I'd be much happier without the request method in there at all or clearly delineating where the derived method column is coming from.

sontek · 2014-11-10T18:23:48Z

@mmerickel Do you have an example where this wouldn't show the correct thing? I could definitely pull the request method out of this PR and make it separate so we can discuss it. But I tried to match the logic up with what Pyramid would do when resolving what methods to allow

mmerickel · 2014-11-10T18:47:30Z

Well in Pyramid the route predicates and view predicates are completely separate, so user's can introduce subtle bugs like route_method='GET' and view_method=('PUT', 'POST') and then wonder why they are getting a 404. If proutes glosses over this issue then it's not really helping the situation. I guess the source of the decision would be useful in the case where both predicates exist?

sontek · 2014-11-10T19:58:18Z

@mmerickel The way I solved this was by only returning methods that could be hit. So in your example PUT and POST would be shown but GET would not. I think its better to at least give the information that GET is not available, rather than masking it and making them wonder why they are getting the 404.

mmerickel · 2014-11-10T20:02:35Z

In my example it's not possible to match request, you will always 404 in either route or view lookup for that route. Also routes can be attached to any view if add_route(..., use_global_views=True) is set so it's really hard to figure out which views are available.

sontek · 2014-11-11T00:22:25Z

You are right, I wrote up a few more conflict examples and made it work a little better:

Name            Pattern                     View                        Method   
----            -------                     ----                        ------   
home            /                           testapp.home                <route mismatch>

But found that sometimes even the current implementation of Pyramid can't resolve what view is attached and returns "unknown", and so sometimes the information could be invalid.

I'm still leaning towards it being useful "most of the time" versus not having it, but I'll re-work the PR without it and the submit a separate PR with request_method so we can discuss it more.

This cleans up the conflict resolving a little more

mmerickel · 2014-11-11T01:09:08Z

Awesome! I think we should totally add the feature as long as it degrades into some sort of "can't do it cap'n" feedback rather than false information in those ambiguous cases.

sontek · 2014-11-11T16:28:13Z

I'll close this one for now and think about request method a bit more.

sontek added 2 commits October 16, 2014 20:35

Make proutes more readable and report more info.

56c3c29

Currently proutes doesn't report what request_method's are accepted and the print out has static padding defined so it becomes unreadable very quickly.

Switch to using __name__

25ae0f3

func_name was deprecated in Python 3

Make proutes prefer the defined request_method on the route

c065154

Prefer PAD=3, saves space and keeps readability

163d812

sontek added 2 commits November 10, 2014 16:37

Pushing this so I have it at home.

9397a48

This cleans up the conflict resolving a little more

Make sure we don't have pdbs

8d683e6

sontek mentioned this pull request Nov 11, 2014

Format proutes output and include module instead of repr of view #1453

Merged

sontek closed this Nov 11, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make proutes more readable and report more info. #1431

Make proutes more readable and report more info. #1431

sontek commented Oct 17, 2014

aconrad commented Oct 17, 2014

stevepiercy commented Oct 17, 2014

mmerickel commented Oct 17, 2014

sontek commented Oct 18, 2014

witsch commented Oct 18, 2014

mmerickel commented Oct 18, 2014

sontek commented Oct 18, 2014

sontek commented Oct 18, 2014

aconrad commented Oct 18, 2014

sontek commented Nov 6, 2014

mmerickel commented Nov 10, 2014

sontek commented Nov 10, 2014

mmerickel commented Nov 10, 2014

sontek commented Nov 10, 2014

mmerickel commented Nov 10, 2014

sontek commented Nov 11, 2014

mmerickel commented Nov 11, 2014

sontek commented Nov 11, 2014

Make proutes more readable and report more info. #1431

Make proutes more readable and report more info. #1431

Conversation

sontek commented Oct 17, 2014

aconrad commented Oct 17, 2014

stevepiercy commented Oct 17, 2014

mmerickel commented Oct 17, 2014

sontek commented Oct 18, 2014

witsch commented Oct 18, 2014

mmerickel commented Oct 18, 2014

sontek commented Oct 18, 2014

sontek commented Oct 18, 2014

aconrad commented Oct 18, 2014

sontek commented Nov 6, 2014

mmerickel commented Nov 10, 2014

sontek commented Nov 10, 2014

mmerickel commented Nov 10, 2014

sontek commented Nov 10, 2014

mmerickel commented Nov 10, 2014

sontek commented Nov 11, 2014

mmerickel commented Nov 11, 2014

sontek commented Nov 11, 2014