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

feat: load check config on startup #115

Merged
merged 15 commits into from
Mar 12, 2024
Merged

feat: load check config on startup #115

merged 15 commits into from
Mar 12, 2024

Conversation

lvlcn-t
Copy link
Collaborator

@lvlcn-t lvlcn-t commented Feb 28, 2024

Motivation

Partly addresses the conversation of #79 to instantly trigger the config load on sparrow startup.

Changes

I've added the initial startup config load and refactored the http loader to return the runtime config per value instead of reference to avoid sending a nil pointer through the channel.

For additional information look at the commits.

Tests done

I've provided some unit tests for the initial config load.

  • Unit tests succeeded.
  • E2E tests succeeded. - we have no e2e tests for this.

TODO

  • I've assigned this PR to myself
  • I've labeled this PR correctly

* feat: add healthz handler
* feat: add health checker component
* feat: check metrics endpoint health
* feat: check health of all check endpoints
* test: add checker tests
* test: add healthz handler tests
@lvlcn-t lvlcn-t added feature Introduces a new feature area/api Issues/PRs related to the API labels Feb 28, 2024
@lvlcn-t lvlcn-t self-assigned this Feb 28, 2024
@lvlcn-t lvlcn-t mentioned this pull request Feb 28, 2024
2 tasks
pkg/sparrow/run.go Outdated Show resolved Hide resolved
pkg/checks/health/health.go Outdated Show resolved Hide resolved
@lvlcn-t lvlcn-t force-pushed the feat/healthz-endpoint branch from 9bb963e to a39f6b9 Compare March 12, 2024 09:30
Copy link
Member

@y-eight y-eight left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine for me.

Still it is not ideal but it is a hard decision.
@niklastreml also mention to give the user multiple options to define a startup & readiness config for the sparrow. At the moment that is overengineereed I think.

@lvlcn-t lvlcn-t changed the title feat: healthz endpoint feat: load check config on startup Mar 12, 2024
@lvlcn-t lvlcn-t merged commit 3e86839 into main Mar 12, 2024
11 checks passed
@lvlcn-t lvlcn-t deleted the feat/healthz-endpoint branch March 12, 2024 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api Issues/PRs related to the API feature Introduces a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants