Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
RFC 72: Background workers #72
RFC 72: Background workers #72
Changes from 2 commits
75d8635
c9e7820
26566bf
71660ae
0e74a5c
d1a9672
5499480
a5f0387
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 a little unclear on the scheduled tasks part of this. Would we replace crons (such as
publish_scheduled_pages
) as Huey periodic tasks, or would they remain as crons?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.
My imagining was both. They'd remain as management commands exactly as they are now, but there'd be the option of triggering them as scheduled tasks through a setting in
settings.py
. I've just realised I've not fleshed out what that API would look like yet.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.
Got it. Is the main benefit for the user that they wouldn't need to set up separate scheduled tasks if they're already running a worker, or are there other benefits you're considering?
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.
Partly that - may save on infrastructure costs. But it also adds the ability for installed packages to "register" scheduled tasks which just get run automatically, without the user having to do anything, or even know / remember they're there.
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.
might be a bit early in the process to think about naming, but maybe we should add a section for proposed names.
I think
task
is already taken up in the context of Workflow/Tasks.Maybe;
job
orworker
But I realise Huey uses Task in its terminology so maybe unavoidable.
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 the name here is definitely just a WIP one to fill a gap.
"worker" feels like it's the thing which performs the task, as opposed to the task itself.
"job" is definitely a common enough term that it could be fine. Agree the conflict with workflows isn't ideal. We could be more explicit and call them "background tasks" or alike?
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.
Perhaps it is the responsibility of the caller of the hook to decide whether it should be run asynchronously?
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.
There may be cases with some hooks where the user wants it to run synchronously, which I've intentionally not ruled out. It will have to be down to the caller of the hook as to whether it's possible for it to be run async, though.
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.
Additional questions
Some relevant links
General feedback
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 suspect not. These tasks are really just an implementation detail, as opposed to anything someone may want to monitor. With that said, I could definitely see a use for a list of scheduled tasks, giving admins the ability to trigger them manually if they wish?
See "Why Huey?" section
I think they can be the same thing. Doing external network requests in the request/response cycle can hurt performance in many cases, so if the webhook doesn't respond immediately it'll slow everything down. Given that, webhooks could definitely be an interesting (future) extension, and probably warrants having background workers regardless.
In an ideal world, yes, although some of the benefits of background workers are in moving parts of wagtail-core into the background, which can't be done with an external package. With that said, integration with things like signals and hooks could definitely be done externally.
One of the main benefits of this solution though is that it's built in. If it's an external package, it just becomes another competing standard.