Central processor registration #229
Conversation
beater/server.go
Outdated
| func newMuxer(config Config, report reporter) *http.ServeMux { | ||
| mux := http.NewServeMux() | ||
|
|
||
| for path, p := range processor.Registry.Processors() { | ||
| handler := appHandler(p, config, report) | ||
| for path, constructor := range Routes { |
There was a problem hiding this comment.
would you mind naming this only c or construc or similar, to avoid having the same name for the type and the variable?
beater/server.go
Outdated
| func processRequest(r *http.Request, p processor.Processor, maxSize int64, report reporter) (int, error) { | ||
| func processRequest(r *http.Request, new constructor, maxSize int64, report reporter) (int, error) { | ||
|
|
||
| var p processor.Processor = new() |
There was a problem hiding this comment.
I like this improvement, to have one dedicated processor per request.
Currently we do call the CreateSchema method though when creating a new processor. With your changes this would be done for every request. I don't think is the way to go. Could you update that the schema is created once per endpoint and then reused per request?
There was a problem hiding this comment.
good point! we totally should do that.
besides, this is not meant to be merged (at least just yet), this conflicts with PR227 and i'd like to see what everyone thinks of #230
There was a problem hiding this comment.
I totally support this improvement, as I think it makes things clearer and less error prone.
beater/server.go
Outdated
| @@ -34,11 +36,18 @@ var ( | |||
| errPOSTRequestOnly = errors.New("only POST requests are supported") | |||
| ) | |||
|
|
|||
| type constructor func() processor.Processor | |||
There was a problem hiding this comment.
how about newProcessor or processorFactory ?
jalvz
force-pushed
the
central-processor-registry
branch
from
3d8f519
to
f70bb1d
Compare
…tadata like endpoint and handlers can be all in one place. This also removes the workarounds with blank identifier imports, and instantiates one processor per request to avoid harmful state
jalvz
force-pushed
the
central-processor-registry
branch
from
f70bb1d
to
d52756e
Compare
|
Rebased and implemented review comments, ready to This undoes parts of #227, now there isn't need for Found no issues with circular imports, on the contrary this removes more package dependencies than adds. |
Register processors centrally, so that processor endpoints and handlers can be all in one place. This also removes the workarounds with blank identifier imports, and instantiates one processor per request to avoid harmful state
Register processors centrally, so that processor "configuration" / metadata like endpoint and handlers can be all in one place
This also removes the workarounds with blank identifier imports, and instantiates one processor per request to avoid harmful state
Fixes #230