Skip to content
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

Deprecate and remove OBSERVE_REQUEST_CALLBACK #1886

Closed
tim-schilling opened this issue Feb 13, 2024 · 6 comments
Closed

Deprecate and remove OBSERVE_REQUEST_CALLBACK #1886

tim-schilling opened this issue Feb 13, 2024 · 6 comments

Comments

@tim-schilling
Copy link
Member

With the addition of UPDATE_ON_FETCH from #1843, we no longer need to keep OBSERVE_REQUEST_CALLBACK around. The functionality that can be implemented with that can be redone in SHOW_TOOLBAR_CALLBACK as indicated by @living180 here

living180 added a commit to living180/django-debug-toolbar that referenced this issue Mar 11, 2024
With the addition of the UPDATE_ON_FETCH setting, the
OBSERVE_REQUEST_CALLBACK setting is redundant.  Thus add a check
debug_toolbar.W008 to warn if it is present in DEBUG_TOOLBAR_CONFIG,
but do not remove it yet.

See django-commons#1886
living180 added a commit to living180/django-debug-toolbar that referenced this issue Mar 11, 2024
With the addition of the UPDATE_ON_FETCH setting, the
OBSERVE_REQUEST_CALLBACK setting is redundant.  Thus add a check
debug_toolbar.W008 to warn if it is present in DEBUG_TOOLBAR_CONFIG,
but do not remove it yet.

See django-commons#1886
living180 added a commit to living180/django-debug-toolbar that referenced this issue Mar 11, 2024
With the addition of the UPDATE_ON_FETCH setting, the
OBSERVE_REQUEST_CALLBACK setting is redundant.  Thus add a check
debug_toolbar.W008 to warn if it is present in DEBUG_TOOLBAR_CONFIG,
but do not remove it yet.

See django-commons#1886
@kevalrajpalknight
Copy link

@tim-schilling this issue could be marked as closed.

@dennisvang
Copy link

(debug_toolbar.W008) The deprecated OBSERVE_REQUEST_CALLBACK setting is present in DEBUG_TOOLBAR_CONFIG.
HINT: Use the UPDATE_ON_FETCH and/or SHOW_TOOLBAR_CALLBACK settings instead.

This deprecation warning appears to suggest that we can remove OBSERVE_REQUEST_CALLBACK from the DEBUG_TOOLBAR_CONFIG dict.

However, that's not the case, because it would break the following:

https://github.com/jazzband/django-debug-toolbar/blob/9e30a06e418ecdd4eeb837530b86be40bb1e3d2d/debug_toolbar/toolbar.py#L182

Moreover, it is not clear to me from the message/docs/PR whether the default values for UPDATE_ON_FETCH and/or SHOW_TOOLBAR_CALLBACK are equivalent to the default value of OBSERVE_REQUEST_CALLBACK.

@tim-schilling
Copy link
Member Author

tim-schilling commented Aug 7, 2024

@dennisvang OBSERVE_REQUEST_CALLBACK defaults to returning [a reference to the function] toolbar.observe_request which returns True. That's why it's safe to remove in a future update of the toolbar.

If you're attempting to have the toolbar reflect the latest request being made, including the AJAX requests while you're on a page, set UPDATE_ON_FETCH to True. If that's not helpful, please provide more information on what you're confused about and what you're trying to do.

@dennisvang
Copy link

dennisvang commented Aug 7, 2024

@tim-schilling Thanks for the explanation.

My point is that we now get a warning if OBSERVE_REQUEST_CALLBACK is in the config dict, but removing it from the config dict causes an error, so it seems like there is no way to act on this warning yet.

@tim-schilling
Copy link
Member Author

tim-schilling commented Aug 7, 2024

@dennisvang It should only log a deprecation warning if you include it in your project's settings' DEBUG_TOOLBAR_CONFIG dict (see https://github.com/jazzband/django-debug-toolbar/blob/9e30a06e418ecdd4eeb837530b86be40bb1e3d2d/debug_toolbar/apps.py#L265). Not including it in that dictionary should be fine (no errors and no warnings).

If you're getting a KeyError, I suspect you're manipulating the toolbar's actual default dict, which you probably shouldn't do unless you have good reason to.

@dennisvang
Copy link

@tim-schilling Thanks, you're right:

It turns out we had DEBUG_TOOLBAR_CONFIG = debug_toolbar.settings.CONFIG_DEFAULTS, without making a copy. So, removing the key from there would indeed manipulate the default dict.

Sorry for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants