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.

Sign up for GitHub

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

Jump to bottom

refactor(api): Add more components to healthcheck – follow-ups #3337

Merged
merged 4 commits into from
Dec 5, 2024
Merged

refactor(api): Add more components to healthcheck – follow-ups #3337

merged 4 commits into from
Dec 5, 2024

Conversation

slowli
Copy link
Contributor

@slowli slowli commented Nov 27, 2024

What ❔

Various minor follow-ups after #3193:

  • Rework app-level health details.
  • Fix execution_time unit of measurement for the database health check details.
  • Rework the database health check: do not hold a DB connection all the time; make it reactive.

Why ❔

Makes the dependency graph lighter; simplifies maintenance.

Checklist

  • PR title corresponds to the body of PR (we generate changelog entries from PRs).
  • Documentation comments have been added / updated.
  • Code has been formatted via zkstack dev fmt and zkstack dev lint.

@slowli slowli marked this pull request as ready for review November 27, 2024 15:43
RomanBrodetski
RomanBrodetski approved these changes Dec 4, 2024
View reviewed changes
Copy link
Collaborator

@RomanBrodetski RomanBrodetski left a comment

Choose a reason for hiding this comment

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

We can discuss the behaviour on intermittent DB issues separately, the PR itself looks good!

manuelmauro
manuelmauro reviewed Dec 4, 2024
View reviewed changes
Copy link
Contributor

@manuelmauro manuelmauro left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks a lot for your previous reviews and these improvements!

@slowli slowli added this pull request to the merge queue Dec 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 4, 2024
@slowli slowli added this pull request to the merge queue Dec 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 4, 2024
@slowli slowli added this pull request to the merge queue Dec 5, 2024
Merged via the queue into main with commit d0078df Dec 5, 2024
34 checks passed
@slowli slowli deleted the aov-add-more-components-to-healthcheck branch December 5, 2024 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@manuelmauro manuelmauro manuelmauro left review comments

@RomanBrodetski RomanBrodetski RomanBrodetski approved these changes

@popzxc popzxc Awaiting requested review from popzxc

@Deniallugo Deniallugo Awaiting requested review from Deniallugo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@slowli @RomanBrodetski @manuelmauro