SHOW_TOOLBAR_CALLBACK no longer supports lambda

[ross]

in my setup i always have seperate dev settings and in those settings i enable debug toolbar for all requests. i've always done that with the following config

DEBUG_TOOLBAR_CONFIG = {
    'SHOW_TOOLBAR_CALLBACK': lambda request: True,
}

after updating my virtualenv this morning i started getting errors that i've tracked down to trying to run rsplit on func_path

line 45 of debu_toolbar/middleware.py:

    mod_path, func_name = func_path.rsplit('.', 1)

it looks like the meaning of SHOW_TOOLBAR_CALLBACK may have changed to require a string and not support a callable. if that's the case i'm not sure what the solution would be. i'd rather not have to define a function somewhere and point this at it just to return true. i'd prefer if SHOW_TOOLBAR_CALLBACK supported being presented a callable.

[aaugustin]

This change was made on purpose to prevent people from copy-pasting the insecure example that was in the old documentation.

Is there a good reason for using this definition? If you simply copy-pasted from the docs without bothering to read the surrounding text, remove it and move on.

[ross]

as i explained in the original description, i'm not just copy-n-pasting.

there isn't a security issue as debug_toolbar isn't even installed on the public/production boxes, much less configured this way. it's in a dev only config file/setup. it's used by developers to run with their local boxes where it's safe/desirable to allow blanket access.

[aaugustin]

All Django settings have primitive values, to avoid the temptation to import stuff in settings. The Debug Toolbar follows this philosophy. I'm sorry that it makes your use case marginally less practical, but I still think it's a good trade-off for the community at large.

I would suggest the following (untested) code:

DEBUG_TOOLBAR_CONFIG = {
    'SHOW_TOOLBAR_CALLBACK': "%s.true" % __name__,
}

def true(request):
    return True

[ross]

that's effectively the work-around i've put in place and it's not that big a deal.

as a general it's a good idea to give a better error message for something that previously worked fine and you're specifically going to break. the following results in users having to open up the source and figure out what went wrong.

AttributeError: 'function' object has no attribute 'rstrip'

something like the following would be preferable.

...
if callable(func_path):
    ImproperlyConfigured('invalid SHOW_TOOLBAR_CALLBACK, a dotted path to a function is required')
...

[aaugustin]

Fair enough!

[ross]

thanks. liking the updated/cleaned up stuff

[tomkins]

Given that the exact same thing can be done as demonstrated above, was there a need to remove the old behaviour? Just feels like it results in a slightly more ugly settings file for my local/dev instances.

My opinion for the best option would be to default to 'debug_toolbar.middleware.show_toolbar' by default as the recommended option in the documentation, with iscallable being used for those of us who actually want it to be a function/lambda.

It's a debug toolbar, I'd hope that anyone installing it would recognise the fact that it can show a lot of sensitive information and that we don't need protecting in this way.

[aaugustin]

You cannot make this assumption. You'd be surprised at how many people run sites in production with DEBUG = True. (More than half of them last time I had the opportunity to check a significant sample.)

:(

[aaugustin]

In fact, I had already included such a warning: https://github.com/django-debug-toolbar/django-debug-toolbar/blob/master/debug_toolbar/settings.py#L66-L69. You should have seen it.

I'm hesitating a bit; I could be convinced to restore support for callables, especially if one of the other maintainers wanted it.

[zsoldosp]

just 2c - the warnings don't show up on the web interface, just in the logs. While it's quick to google (http://stackoverflow.com/a/20727979), but it would be nice to have a the same error message on the UI as it is in the warnings.