Open albrow opened 9 years ago
+1 I like this idea. I was also turned off by HandlerFunc
being an interface{}
which required reflection.
Also what do you think about exposing job details to the Execute func? For example the job id and name might be useful in some cases(ie smartly rescheduling the current job based on the results it got).
@epelc thanks for the feedback!
With the implementation described above, the job details are already exposed in the Execute
method because DefaultJob
is embedded in EmailJob
. So if you needed access to the id, status, etc.:
func (j EmailJob) Execute() error {
id = j.JobId()
// ...
}
+1 this proposal as well, very idiomatic and feels right
+1, finding it difficult to implement with current examples / ideology on the README
I would like to change the public API in a pretty major way by using an interface for jobs instead of a handler function (which is not typesafe). The current implementation feels a little bit messy and unidiomatic to me.
Jobs follows semantic versioning, which means breaking changes are fair game until version 1.0. However, I never put a warning in the README about this, so I wanted to make sure it was okay with everyone before making this breaking change. I would also be happy to hear feedback on the approach.
The basic idea is to create an interface that might look something like this:
Most of these have straightforward implementations which could be covered with an embeddable
DefaultJob
orJobData
type, similar to the approach I use in zoom.So job type declarations would now look like this:
I can leverage zoom as a library to easily serialize all the exported fields in any job struct. So when a job gets retrieved from the database, all the struct fields will be filled in. Then the worker will just call the
Execute
method.There are a few advantages to this approach:
DefaultJob
or provide your own implementations of the methods needed, or if you don't define an Execute method, the compiler will tell you, whereas previously these types of omissions would be runtime errors. Workers can also execute jobs by calling theExecute
method instead of jumping through fiery hoops with reflection.RegisterJob
just feels wrong.Let me know what you think. If I don't hear any objections I'll plan on converting to the new implementation sometime in the coming weeks.