refactor: introduces bootstrapper to orchestrate server initialization #23
Conversation
http/pprof/service.go
Outdated
| @@ -0,0 +1,26 @@ | |||
| package pprof | |||
There was a problem hiding this comment.
Why can't we use the same http server for pprof as used for WebSocket/REST?
There was a problem hiding this comment.
We can. I was just following how is it right now. I haven't made changes here. We can use the same HTTP server as long as the path is not conflicting.
There was a problem hiding this comment.
Actually, there is a bug in the current implementation. We haven't defined the paths explicitly, which is a requirement when using gorilla.
http/services.go
Outdated
| } | ||
| } | ||
|
|
||
| func Create(b chan *collection.CollectRequest) Services { |
There was a problem hiding this comment.
Can we have a better name for this function? This function will be called http.Create, for someone who is reading the function call it will very difficult to access what is being created.
There was a problem hiding this comment.
Do you have suggestions? What about servers or services?
There was a problem hiding this comment.
or handlers. handlers sounds good. handlers.create inside handlers there are Websocket handler, REST HTTP handler, GRPC handler, etc.
There was a problem hiding this comment.
Services is what sounds right to me as it returns the created services.
http/services.go
Outdated
|
|
||
| type bootstrapper interface { | ||
| // Init initialize each HTTP based server. Return error if initialization failed. Put the Serve() function as return mostly suffice for Init process. | ||
| Init() error |
There was a problem hiding this comment.
Can we pass context as a function param here too? Just to make it standard.
http/services.go
Outdated
| type bootstrapper interface { | ||
| // Init initialize each HTTP based server. Return error if initialization failed. Put the Serve() function as return mostly suffice for Init process. | ||
| Init() error | ||
| Shutdown(ctx context.Context) |
http/services.go
Outdated
| } | ||
| } | ||
|
|
||
| func Create(b chan *collection.CollectRequest) Services { |
There was a problem hiding this comment.
Just a thought - if we want to initialize only a certain server in the future may be based on a config param, will it make sense to move this logic outside this package. In main maybe?
There was a problem hiding this comment.
Yes. This we can refactor in the future. This attempt is only the first attempt to segregate between handlers protocol.
http/rest/service.go
Outdated
| ) | ||
|
|
||
| type Service struct { | ||
| Buffer chan *collection.CollectRequest |
There was a problem hiding this comment.
Can we make the collector as a struct field instead of the channel, currently we are initializing collector the collector twice whereas only one can be used?
No description provided.