mirror of
https://github.com/hibiken/asynq.git
synced 2026-04-26 07:25:56 +03:00
[GH-ISSUE #326] [FEATURE REQUEST] Introduce Task Locals #2165
Labels
No labels
CLI
bug
designing
documentation
duplicate
enhancement
good first issue
good first issue
help wanted
idea
invalid
investigate
needs-more-info
performance
pr-welcome
pull-request
question
wontfix
work in progress
work in progress
work-around-available
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/asynq#2165
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @hibiken on GitHub (Sep 14, 2021).
Original GitHub issue: https://github.com/hibiken/asynq/issues/326
Originally assigned to: @hibiken on GitHub.
Is your feature request related to a problem? Please describe.
In some cases, we have middleware functions that want to add more context or decorate task so that downstream handler functions can retrieve that data. Similar issues: #285 #313
Describe the solution you'd like
To allow users to store local data within a handling of task, we can provide
map[string]interface{}as "task locals".Task locals is not persisted so any modification to the map doesn't persist across retries or locals set before enqueue is not visible to the handler code. This local should be used within the same
Handler.ProcessTaskscope.Introduce the following method on
TaskDescribe alternatives you've considered
#313 : However, I'm worried that this API will confuse some users. Additionally, the reason
net/httppackage embedded the context within theRequeststruct is to avoid breaking changes in their API.Additional context
None for now.
@crossworth commented on GitHub (Sep 14, 2021):
Hello, I have thought more about #313 and think that the context should be the first parameter as well, I dont think we should "hide" it inside a struct, go docs/context.Context clearly says:
The problem that #313 tries to solve is a way to change the context inside a task handler, the only reason is to allow context propagation (and all the patterns that is has already, like scoped values, tracing).
If we think that the task handler is the last step on the "task processing", this problem would not exist, but in some cases there is a need for an additional step, so a context propagation from the task handler is needed.
I think this PR solves the problem of returning a value from the task to the middleware, but I'm not sure if this solves the context propagation problem.
So I don't know a easy way to allow context propagation.
One pattern that we could make work (I think) is using a custom context, context is already an interface, we could create an
TaskContextthat implement the context interface and have more functions to it. Users that need the concrete type/functions could do a interface upgrade.This should be fully backwards compatible, I just don't know if it something that we want to do.
I will close #313 since I dont think is a good solution.
I have give more thought about this PR. I think this a good solution, ofcourse I would like a way to share the context with the middleware back from a handler, but I guess there is ways we can achieve if we really need (on the consumer side of the library, like a middleware that use a custom context, using Locals or a global shared state). I would like to know how would the context be shared/handled when using pipelines/workflows.
If the workflow implementation allow some support of context sharing or job batching it would be cool to allow to share the same context or provide a workflow context with the task context.
@dhh93 commented on GitHub (Sep 15, 2021):
@hibiken +1 to that implementation.
I've tried implementing message passing/data sharing in middleware functions -- either by mutating the task payload directly or by wrapping handlers in a custom struct and passing data thru a field. The problem with the latter approach is that it is best meant for sharing global data as the data would be available to all tasks processed by that handler. The new method would best address cases in which you'd want to share one-off data with a specific task.
My only (minor) feedback if it could be named
Metadatainstead ofLocalsfor more clarity 😅 .@hibiken commented on GitHub (Jan 16, 2022):
Closing this issue for now since I don't think this is a common need.