goblero icon indicating copy to clipboard operation
goblero copied to clipboard

Add ctx to job processing

Open didil opened this issue 6 years ago • 2 comments

didil avatar Feb 13 '19 02:02 didil

I think we can achieve this by adding ctx as the first parameter for method Run() of Processor. Something like this

type Processor interface {
	Run(ctx context.Context, j *Job) error
}

Is this what you mean? If yes, I can help with the issue

xamenyap avatar Mar 03 '19 02:03 xamenyap

I thought maybe we could find a way to make ctx optional/ implicit, similarly to how its done in the http std library https://golang.org/pkg/net/http/#Request.Context ? then make ctx a field on the Job, ctx context.Context

any thoughts ?

didil avatar Mar 06 '19 01:03 didil

I thought maybe we could find a way to make ctx optional/ implicit, similarly to how its done in the http std library https://golang.org/pkg/net/http/#Request.Context ? then make ctx a field on the Job, ctx context.Context

any thoughts ?

Sure, that works too. Probably it's better to provide another method to *Blero to enqueue a job with Context, similar to how std lib provides http.NewRequest and http.NewRequestWithContext

// uses a default context.Background()
func (bl *Blero) EnqueueJob(name string, data []byte) (uint64, error) {
  jID, err := bl.queue.enqueueJob(context.Background(), name, data)
}

// use the provided context
func (bl *Blero) EnqueueJobWithContext(ctx context.Context, name string, data []byte) (uint64, error) {
  jID, err := bl.queue.enqueueJob(ctx, name, data)
}

Job context can be used like this in dispatcher

func (d *dispatcher) runJob(q *queue, pID int, p Processor, j *Job) {
	defer d.processorDone(pID)
	select {
	case <-j.ctx.Done():
		err := q.markJobDone(j.ID, jobFailed)
		if err != nil {
			fmt.Printf("markJobDone -> %v jobFailed failed: %v\n", j.ID, err)
		}
	default:
		// run the job
	}
}

xamenyap avatar Nov 24 '22 09:11 xamenyap

thanks @xamenyap for the input, I'm closing this issue for now, it does not seem to be a priority at the moment

didil avatar Nov 25 '22 08:11 didil