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!: Remove interior mutability of HealthCheckRegistry #258

Merged
merged 1 commit into from
Jul 1, 2024

Conversation

spencewenski
Copy link
Member

@spencewenski spencewenski commented Jul 1, 2024

Switch to using OnceLock to allow setting the health checks on the AppContext after it is created. The method to set the health checks is private to the crate, so we have control over when the checks are set on the context.

This has some impacts on public-facing APIs:

  • App::health_checks takes a &mut HealthCheckRegistry instead of a non-mutable ref
  • AppContext::health_checks returns the list of health checks. (Alternatively, we could have returned an Option)
  • We also made the HealthCheckRegistry::new method non-public. This aligns with the ServiceRegistry::new visibility.

Closes #257

Switch to using `OnceLock` to allow setting the health checks on the
`AppContext` after it is created. The method to set the health checks is
private to the crate, so we have control over when the checks are set on
the context.

This has some impacts on public-facing APIs:
- `App::health_checks` takes a `&mut HealthCheckRegistry` instead of a
  non-mutable ref
- `AppContext::health_checks` returns the list of health checks.
  (Alternatively, we could have returned an Option)
- We also made the `HealthCheckRegistry::new` method non-public. This
  aligns with the `ServiceRegistry::new` visibility.
Copy link

codecov bot commented Jul 1, 2024

Codecov Report

Attention: Patch coverage is 42.85714% with 24 lines in your changes missing coverage. Please review.

Project coverage is 44.09%. Comparing base (bdd9458) to head (b2e308e).

Files Coverage Δ
src/health_check/default.rs 100.00% <100.00%> (ø)
src/health_check/registry.rs 96.66% <100.00%> (+9.16%) ⬆️
src/api/core/health.rs 0.00% <0.00%> (ø)
src/app/mod.rs 0.00% <0.00%> (ø)
src/app/context.rs 23.07% <0.00%> (-3.02%) ⬇️

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bdd9458...b2e308e. Read the comment docs.

@spencewenski spencewenski merged commit eee057f into main Jul 1, 2024
14 of 16 checks passed
@spencewenski spencewenski deleted the gh-257-once-lock branch July 1, 2024 18:43
@github-actions github-actions bot mentioned this pull request Jul 1, 2024
spencewenski pushed a commit that referenced this pull request Jul 1, 2024
## 🤖 New release
* `roadster`: 0.4.0 -> 0.5.0

<details><summary><i><b>Changelog</b></i></summary><p>

<blockquote>

##
[0.5.0](roadster-v0.4.0...roadster-v0.5.0)
- 2024-07-01

### Added
- [**breaking**] Remove interior mutability of `HealthCheckRegistry`
([#258](#258))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/MarcoIeni/release-plz/).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

Use OnceLock to late-init the health check registry in the context?
1 participant