search

warpfork / go-sup

Supervisor trees for Golang!
MIT License
4 stars 1 forks source link

Thoughts on suture pkg #1

Open lanzafame opened 5 years ago

lanzafame commented 5 years ago

I am wondering what your thoughts are on https://github.com/thejerf/suture are and whether there are any major design differences between the two packages?

warpfork commented 5 years ago

I looked at it a long time ago and had difference of opinion, but can't instantly recall all of them and didn't keep good notes :thinking:

I think it came down to that I'm a bit dubious of the type Service interface { Serve(); Stop(); } core. That mental model is based on actors; and I wanted to build supervision... without the innate belief in actor models.

(Mind, I still like actor models. I build them all the time. But I think we can make supervision orthagonal to that.)

Another probably significant difference is that this go-sup package uses standard library's context.Context heavily, both for passing any metadata (e.g. task tree names) around transparently, and for the obviously important cancellation and halting purpose. I've actually (re)written something "like" this library several times, and the first ones didn't lean on context; this one does, and it's come out the nicest so far (IMO) and also integrates easily with other parts of the Go ecosystem which are now all standardizing around context.

Also, if suture is considered matured and unlikely to change, that's definitely a difference :) I definitely expect to make at least one more significant change to this go-sup package (mostly having to do with stream supervisors, though; forkjoin style probably won't change much).

warpfork commented 5 years ago

That was an off-the-cuff answer, but I think it mostly checks out. I'll leave this issue open to remind myself to write better docs about this, though. This sort of stuff should probably be boiled down into the readme or some other docs in-repo.

lanzafame commented 5 years ago

@warpfork Thanks for the response.

The use of context.Context, though I wish it wasn't the case, is necessary for any sort of tracing and metrics collection framework these days in Go, so yeah that is a deal breaker for suture, for me at least.

warpfork commented 5 years ago

Yeah it took me a long time to come around on context.Context too. But now that I've seen it used for opentracing, I've totally conceded -- long live our new Context overlords.