accept custom encoder for payload#135
Conversation
task.go
Outdated
| rawPayload: payload, | ||
| } | ||
|
|
||
| opts = append(opts, TaskPayloadEncoder(json.Marshal)) |
There was a problem hiding this comment.
this seems a bit magical and hard to decipher.
I think it would be clearer if Task had a unexpected key marshaller that held a TaskPayloadEncoderFunc - and defaults to the json one.
This way we can have the option just set the marshaller and then call the marshaller here.
You can remove the magical "first marshaller is used" thing by just checking its nil before setting it and erroring when not nil, thats a bit more explicit.
Finally if its nil after opts parsing then set json one and use it after.
There was a problem hiding this comment.
With this approach, users can provide multiple TaskPayloadEncoderFunc options without causing multiple encodings, since these options only set the encoder function during task creation. The actual encoding happens once, after all options are processed. This means we can drop the current restriction that only the first encoder is used and instead follow the more common pattern where the last option set takes precedence.
What do you think?
There was a problem hiding this comment.
Yes, thats what I ment. But even better since you can error if more than one marshaller is given if you check the marshaller is nil before setting it.
You can remove the magical "first marshaller is used" thing by just checking its nil before setting it and erroring when not nil, thats a bit more explicit.
ripienaar
left a comment
ripienaar
left a comment
There was a problem hiding this comment.
nice, this looks good thank you
2 participants
A Custom Task Payload Encoder can be provided using
TaskPayloadEncoderTaskOpt. The default behaviour ofjson.Marshalis kept, ifTaskPayloadEncoderis not provided. Only the first provided encoder is used to marshal, rest are discarded, because it does not make sense that the user would want to encode same payload multiple times with no effects. This behaviour is documented.I did not proceed with a separate API mainly due to the need of duplicate logic of creating new task, which would result in extracting all the code of
NewTaskin a non-exported function, except thejson.Marshal(payload)part.