Conversation
|
|
||
|
|
||
| @task | ||
| def send_welcome_message(message): |
There was a problem hiding this comment.
I would suggest setting up some tasks that follow the Django examples tasks closely:
https://docs.djangoproject.com/en/dev/topics/tasks/
That will help it feel a bit more 'real-world' and may inspire further ideas for functionality in the panel itself :-)
| @@ -20,6 +21,13 @@ def jinja2_view(request): | |||
|
|
|||
|
|
|||
| async def async_home(request): | |||
There was a problem hiding this comment.
I'm not super familiar with this codebase, but when does this view get used?
I'd defer to @tim-schilling and others, but it may make sense to have a dedicated view for triggering off example tasks vs, say, triggering every time you visit an example home page. That could also be a bit more self-documenting and give a place to build off of for the future (like testing larger queues, reporting results on a page template, etc)
There was a problem hiding this comment.
Yeah, it makes sense to have a separate view to test the tasks integration.
| try: | ||
| from django.tasks import task | ||
| except ImportError: | ||
| # Define a fallback decorator |
There was a problem hiding this comment.
What are the cases where this could happen? I'm guessing just in older django versions?
Is a fallback decorator a common pattern in this case? I think if its an example meant to work with tasks it shouldn't need a fallback, but I'd be interested to hear more of your thought process. :-)
There was a problem hiding this comment.
We have generally started preferring django.VERSION comparisons to handling ImportError in these cases since old branches can automatically be removed by django-upgrade in the former case.
| "debug_toolbar.panels.cache.CachePanel", | ||
| "debug_toolbar.panels.signals.SignalsPanel", | ||
| "debug_toolbar.panels.community.CommunityPanel", | ||
| "debug_toolbar.panels.tasks.TasksPanel", |
There was a problem hiding this comment.
@tim-schilling I wonder if it would make sense to check for the Django version here, and only include it if its on a Django version that supports the tasks?
There was a problem hiding this comment.
It could, but it's possible someone installs django-tasks in <6.0 version.
f30889c to
6a1a90c
Compare
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
tim-schilling
left a comment
There was a problem hiding this comment.
Thank you for continuing to work on this! I'm liking how it's turning out.
| try: | ||
| from django.tasks import Task | ||
|
|
||
| if hasattr(self, "_original_enqueue"): | ||
| Task.enqueue = self._original_enqueue | ||
| except (ImportError, AttributeError): | ||
| pass |
There was a problem hiding this comment.
I'm not sure we should try to unwrap the method here in case something else has also wrapped it because that would be unwrapped too. Instead we could have the wrapped function call _record_task and then set that function to None, sort like how the CachePanel wraps the cache methods.
Another option is to use a Django signal and connect/disconnect from the signal handler sort of like the TemplatesPanel. Though we'd still need to do the monkey patching.
Give those two panel's patching methods a look and let me know if you have any questions.
|
Hi @tim-schilling |
Description
This PR creates a panel for Django Tasks to allow the Debug Toolbar to monitor queued tasks.
Fixes #2204
Checklist:
docs/changes.rst.