-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Feature/task panel #2235
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
base: main
Are you sure you want to change the base?
Feature/task panel #2235
Conversation
|
|
||
|
|
||
| @task | ||
| def send_welcome_message(message): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 :-)
| return render(request, "index.jinja", {"foo": "bar"}, using="jinja2") | ||
|
|
||
|
|
||
| async def async_home(request): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could, but it's possible someone installs django-tasks in <6.0 version.
robhudson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great progress!
f30889c to
6a1a90c
Compare
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
Description
This PR creates a panel for Django Tasks to allow the Debug Toolbar to monitor queued tasks.
Fixes #2204
Checklist:
docs/changes.rst.