Skip to content
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

improvement: Server.WithHealth supports multiple calls by appending #279

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

bmoylan
Copy link
Contributor

@bmoylan bmoylan commented Apr 1, 2021

Previously, all user-provided healthchecks must be provided in a single call to WithHealth. This has forced applications to build their own healthcheck accumulators.

I have written this code in multiple repos:

server.New().
WithInitFunc(func(ctx context.Context, info witchcraft.InitInfo) (cleanup func(), rErr error) {
	var healthchecks []status.HealthCheckSource
	registerHealth := func(s status.HealthCheckSource) { healthchecks = append(healthchecks, s) }
	defer info.Router.WithHealth(healthchecks...)

	// use registerHealth as a variable to pass to other constructors or things that produce a healthcheck
}

This behavior change is technically a break if anyone is relying on overwriting the values, but I have not been able to find and example of that use case. I considered adding a new API for this (AppendHealth?) but decided the API bloat wasn't worth it. Happy to reconsider if others disagree.


This change is Reviewable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant