Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Deprecating INTERCEPT_REDIRECTS in favor of DEFAULT_DISABLED_PANELS. #548

Merged
merged 3 commits into from Mar 9, 2014

Conversation

Projects
None yet
2 participants
Contributor

tim-schilling commented Feb 14, 2014

Adding ability to disable panels by config setting for #510.

@aaugustin aaugustin commented on an outdated diff Feb 15, 2014

docs/configuration.rst
@@ -57,6 +57,13 @@ toolbar itself, others are specific to some panels.
Toolbar options
~~~~~~~~~~~~~~~
+* ``DEFAULT_DISABLED_PANELS``
+
+ Default: ``('RedirectsPanel', )``
@aaugustin

aaugustin Feb 15, 2014

Contributor

Could you use fully-qualified identifiers as in DEBUG_TOOLBAR_PANELS?

It's technically possible to have two panels with the same class name in different modules. That doesn't happen with built-in panels, but it could happen with external panels.

@aaugustin aaugustin commented on an outdated diff Feb 15, 2014

debug_toolbar/settings.py
@@ -125,6 +125,23 @@
PANELS[index] = new_panel
+if 'INTERCEPT_REDIRECTS' in USER_CONFIG:
+ warnings.warn(
+ "INTERCEPT_REDIRECTS is deprecated. Please use the "
+ "DEFAULT_DISABLED_PANELS config in the"
+ "DEBUG_TOOLBAR_CONFIG setting.", DeprecationWarning)
+ if USER_CONFIG['INTERCEPT_REDIRECTS']:
+ if 'RedirectsPanel' in CONFIG['DEFAULT_DISABLED_PANELS']:
+ # RedirectsPanel should be enabled
+ CONFIG['DEFAULT_DISABLED_PANELS'] = [
+ panel for panel in CONFIG['DEFAULT_DISABLED_PANELS']
+ if panel != "RedirectsPanel"
+ ]
+ elif not 'RedirectsPanel' in CONFIG['DEFAULT_DISABLED_PANELS']:
+ # RedirectsPanel should be disabled
+ CONFIG['DEFAULT_DISABLED_PANELS'].append('RedirectsPanel')
@aaugustin

aaugustin Feb 15, 2014

Contributor

The default value, which is documented, is a tuple. And you cannot append to tuples.

Since all we're doing is testing for inclusion, you could cast DEFAULT_DISABLED_PANELS = set(DEFAULT_DISABLED_PANELS), which would make it easier to add or remove items with simple set operations (add / delete if I remember correctly).

Contributor

aaugustin commented Feb 15, 2014

Thanks for the pull request, it looks pretty good. Can you adjust it according to my comments?

Contributor

tim-schilling commented Feb 15, 2014

@aaugustin Done. However, I did make one more adjustment. I made the definition use a set rather than a tuple. It made more sense to just have it always be a set as there's no need for ordering or duplicates in the collection. Thanks for the thorough review!

@aaugustin aaugustin added a commit that referenced this pull request Mar 9, 2014

@aaugustin aaugustin Merge pull request #548 from tim-schilling/disable-panels-by-default-510
Deprecating INTERCEPT_REDIRECTS in favor of DEFAULT_DISABLED_PANELS.
88712cf

@aaugustin aaugustin merged commit 88712cf into jazzband:master Mar 9, 2014

1 check passed

default The Travis CI build passed
Details
Contributor

aaugustin commented Mar 9, 2014

FYI I changed the name of the setting and tweaked the documentation.

@ryneeverett ryneeverett pushed a commit to ryneeverett/django-debug-toolbar that referenced this pull request Oct 2, 2016

@aaugustin aaugustin Merge pull request #548 from tim-schilling/disable-panels-by-default-510
Deprecating INTERCEPT_REDIRECTS in favor of DEFAULT_DISABLED_PANELS.
d27862f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment