Proposal: Provide context.Context via goka.Context about goka HOT 7 CLOSED

I agree that we need to pass the context.Context somehow up to the ProcessCallback. Otherwise one needs to hack it via closure.

Another problem of extending the goka.Context interface is that it conflicts with the context.Context() in the Value() method.

Do you think that returning the context with ctx.Context() or ctx.Background() would be an alternative? Too confusing? Otherwise we'd need to rename the Value() method.

from goka.

You're right about the conflict in the method name! If backward compatibility is not an issue, Value could be renamed. Although I think we could do something similar to what net/http did for Request:

1. Add Context method to goka.Context. Calling this method would return context.Context passed to the processor's Run method.
2. Add WithContext method to goka.Context. It would accept context.Context and would return shallow copy of goka.Context. Like that: https://godoc.org/net/http#Request.WithContext This is useful for creating child context off of processor's context. For example to add some trace id for a particular message or stuff like this.

I think this kind of change would be the least destructive.

from goka.

I forgot to mention that step 0 would be to insert processor's context.Context inside goka.Context, that further would be available with Context() method on goka.Context.

from goka.

Steps 0 and 1 are very easy, I can quickly build them. But I am not very sure how one would use the WithContext method. Can you give an example?

Looking forward, after closing this ticket with a non-destructive solution, I'd suggest we start discussing how the callback interface could be redesigned. We can plan an API breaking release if we pack enough changes together, for example, #108.

from goka.

WithContext could be useful if you isolate the logic in some sort of Service struct or separate function.

For example I commonly implement the handling in some sort of service type, and then wrap it inside DefineGroup call.

Here is a rough example:

func NewProcessor(brokers []string, svc *FooService, opts ...goka.ProcessorOption) {
	goka.DefineGroup("group", goka.Input("topic1", new(myCodec), func(ctx goka.Context, message interface{}) {
		// Here is when we could derive context if needed...
		stdctx := addTracingInformationOrAnythingElse(ctx.Context())
		ctx = ctx.WithContext(stdctx)

		if err := svc.HandleFooMessage(ctx, message.(*FooMessage)); err != nil {
			// Sometimes not all errors should trigger panic, so we can handle this specific errors in one place here if needed.
			// ...

			ctx.Fail(err)
		}
	}))
}

type FooService struct {
	Log Logger
	DB Database
	Client SomeHTTPClient
	// ...
}

type FooMessage struct {
	FieldOne string
	FieldTwo string
	// ...
}

func (fs *FooService) HandleFooMessage(ctx goka.Context, message *FooMessage) error {
	// Here is where one could use the underlying context.
	fs.Client.Do(ctx.Context(), "rest of parameters here")

	// Handle specific type of *FooMessage.
	// Don't use ctx.Fail here, and instead return normal error.
	// This is more portable and easier to test if needed to test only the logic.
	return nil
}

from goka.

Totally agree on that. We could group enough breaking changes before doing them.

Looking forward, after closing this ticket with a non-destructive solution, I'd suggest we start discussing how the callback interface could be redesigned. We can plan an API breaking release if we pack enough changes together, for example, #108.

from goka.

Let's close this and have another discussion if the issue comes up again.

from goka.