-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
pkg/services: add services.Config.NewServices/Internals helper #204
base: main
Are you sure you want to change the base?
Conversation
104050e
to
a48a03c
Compare
a48a03c
to
47c53ca
Compare
47c53ca
to
41aac5f
Compare
41aac5f
to
055b33b
Compare
055b33b
to
4eaf9fe
Compare
return | ||
} | ||
|
||
// Output: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a txtar like way to test against this output? That might be too brittle since the sub services are closed concurrently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a txtar like way to test against this output?
It is already built-in. These execute like regular tests.
https://go.dev/blog/examples
That might be too brittle since the sub services are closed concurrently.
We can probably add a test-only hack to make the close deterministic 🤔
4eaf9fe
to
1422cad
Compare
1422cad
to
7bb4356
Compare
return | ||
} | ||
|
||
func (s *service) HealthReport() map[string]error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a fail-safe, we could track when this is called, and if it is never called, or not called often enough, then we could periodically emit to logs instead 🤔
7bb4356
to
6bb7b30
Compare
6bb7b30
to
a5aae0e
Compare
func (e *Engine) HealthEvent(err error) { e.healthEvent(err) } | ||
|
||
// SetUnhealthy records a condition key and an error, which causes an unhealthy report, until SetHealthy(condition) is called. | ||
func (e *Engine) SetUnhealthy(condition string, err error) { | ||
e.condsMu.Lock() | ||
defer e.condsMu.Unlock() | ||
e.conds[condition] = fmt.Errorf("%s: %e", condition, err) | ||
} | ||
|
||
// SetHealthy removes a condition and error recorded by SetUnhealthy. | ||
func (e *Engine) SetHealthy(condition string) { | ||
e.condsMu.Lock() | ||
defer e.condsMu.Unlock() | ||
delete(e.conds, condition) | ||
} | ||
|
||
// Unhealthy causes an unhealthy report, until the returned func() is called. | ||
// Use this for simple cases where the func() can be kept in scope, and prefer to defer it inline if possible: | ||
// | ||
// defer Unhealthy(fmt.Errorf("foo bar: %i", err))() | ||
// | ||
// See SetUnhealthy for an alternative API. | ||
func (e *Engine) Unhealthy(err error) func() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am still not satisfied with these names. I think that these four different operations are fundamental, but I'm not sure what to call them that would be more intuitive than this... 🤔
}) | ||
} | ||
|
||
func (s *service) Close() error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DeividasK We could prioritize Stop while adapting Close easily here without spamming it in every implementation.
func (s *service) Close() error { | |
func (s *service) Close() error { return s.Stop() } | |
func (s *service) Stop() error { |
This PR introduces
services.Config.NewServices
withservices.Internals
to help createservices.Service
s in a standard way that reduces boilerplate and the risk of mistakes that comes with re-implementing logger naming, health reporting, sub-service management, etc.Blocked by: