configurable request_history params

[jvanasco]

This is regarding #205

I have a few notes:

1. I did some light housekeeping and alphabetized the imports and some settings in __init__.py; it was starting to get hard to understand what was/wasn't imported.
2. I added a new function to utils -- get_application_registry. An issue I ran into, is that the actual settings are located in two distinct areas, depending on where you access the request.

• The setup routines are called by the "end-user's" application. The settings are in request.registry.settings.
• The view routines are called within a "child" application (i guess due to the toolbar being a separate item via config.make_wsgi_app()). The settings are still located in the "end-user's" application, but this is now in request.registry.parent_registry.settings

The button_style in request_view was broken because of this (now fixed).

1. I enabled ToolbarStorage to have a configurable default too. All current requests to it specify the exact number, but there was a default already so i thought it should be configurable.

[mmerickel]

Ok I had a chance to review this and it looks like it's about ready to be merged. I have a few requests.

1. Add the settings to pyramid_debugtoolbar.default_settings and they will be automatically injected into the debugtoolbar's registry and you won't need to keep doing registry.parent_registry.settings everywhere.

2. Avoid parsing the settings in several spots. Just parse it once and overwrite the setting with the python value or default. Then just always get the value from the settings. The parsed/converted value is already stored in the settings like this if you do step 1.

3. The default_last option added to ToolbarStorage is redundant with the fact that every time you are using .last you have the settings available. I'd remove it and make the num_items argument to last required with no default.

4. Rename show_request_history to max_visible_requests or something to make it more clear that it is a number. Currently it reads as if it's a boolean.

5. I'm not sure I see a need for get_application_registry given these points.

[jvanasco]

That all makes sense. Will put this on my to-do list, then comment when it's done.

[jvanasco]

I integrated the requests. Can rebase to squash the two commits into one, if that's better.

The big things to note:

• we now need an as_int conversion for the default values of max_request_history & max_visible_requests in __init__.py. This is handled by `utils.as_int, which just converts a string to int.

• we also need to handle None as a conversion method for the default value of button_style in __init__.py. (without being in the list of defaults, the setting won't be saved to the right registry). This is handled by conditionally converting the value in the the nested populate def of parse_settings. I also added a few docstrings to parse_settings and the lists of default_settings & default_transform for clarity.

[mmerickel]

Would you mind merging in the latest master? I'd pushed some changes previously for the debugtoolbar.includes feature which is causing some conflicts. I apologize in advance!

[jvanasco]

sure. on it now.

[jvanasco]

So I've got this mostly working in 48323c8

I updated the tests to handle my defaults and the new includes

The only failure right now is in test_toolbar.py / test_it_path_cannot_be_decoded

This is broken in trunk too.

The problem is that the request doesn't have a registry in this view (and it's trying to pull the registry.settings). I'm not sure why this request doesn't have a registry though.

[mmerickel]

All tests pass on my mac on both branches. Can you paste the traceback?

edit sorry I lied, they pass on your branch but not on master which is good enough for me.

[jvanasco]

This is the error I'm getting off Pylons/pyramid_debugtoolbar master:

python setup.py test

======================================================================
ERROR: test_it_path_cannot_be_decoded (pyramid_debugtoolbar.tests.test_toolbar.Test_toolbar_handler)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jvanasco/webserver/os-projects-others/pyramid_debugtoolbar/pyramid_debugtoolbar/tests/test_toolbar.py", line 172, in test_it_path_cannot_be_decoded
    self.assertRaises(URLDecodeError, self._callFUT, request)
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/unittest/case.py", line 475, in assertRaises
    callableObj(*args, **kwargs)
  File "/Users/jvanasco/webserver/os-projects-others/pyramid_debugtoolbar/pyramid_debugtoolbar/tests/test_toolbar.py", line 166, in _callFUT
    return handler(request)
  File "/Users/jvanasco/webserver/os-projects-others/pyramid_debugtoolbar/pyramid_debugtoolbar/toolbar.py", line 248, in toolbar_tween
    toolbar.inject(request, response)
  File "/Users/jvanasco/webserver/os-projects-others/pyramid_debugtoolbar/pyramid_debugtoolbar/toolbar.py", line 82, in inject
    button_style = get_setting(request.registry.settings,
AttributeError: 'Request' object has no attribute 'registry'

======================================================================
FAIL: test_it (pyramid_debugtoolbar.tests.test_init.Test_parse_settings)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jvanasco/webserver/os-projects-others/pyramid_debugtoolbar/pyramid_debugtoolbar/tests/test_init.py", line 48, in test_it
    'debugtoolbar.prevent_http_cache': False,
AssertionError: {'debugtoolbar.intercept_redirects': False, 'debugtoolbar.hosts': ['127.0.0.1'], [truncated]... != {'debugtoolbar.intercept_redirects': False, 'debugtoolbar.debug_notfound': False [truncated]...
Diff is 1070 characters long. Set self.maxDiff to None to see it.

I fixed the second failure (test_it) in my branch.

[mmerickel]

The only error I see is with the test_parse_settings which you've fixed in the PR, which is due to me not properly testing the includes feature. As far as the next test, you should update the test to do request.registry = self.config.registry.

[jvanasco]

The pull is passing in Travis, so fixing the test doesn't look right.

It seems like I had a bad pyramid install. The env i was using still had 1.3a6. It was failing on that test (and only that test). On a fresh env with 1.5.2, everything passed. I tried to figure out what the minimum pyramid version should be, and all the 1.3.x pass (even the 1.3a6 that was broken on mine)

The 1.2 branch has multiple failures on the testsuite FAILED (failures=1, errors=37) It might make sense to bump up the minimum version from pyramid>=1.2dev to pyramid>=1.3. I can put that in a new ticket.

[mmerickel]

Good catch, maybe we should be sure things are still kosher with older versions. I doubt anyone has been doing that work.

Thank you for this PR, merged it now.