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

Descriptive stream & consumer health errors #6416

Merged
merged 1 commit into from
Jan 27, 2025

Conversation

neilalexander
Copy link
Member

This should make the output of healthz less opaque when there are stream or consumer healthcheck errors. Also more closely aligns the checks between streams and consumers.

Signed-off-by: Neil Twigg [email protected]

@neilalexander neilalexander requested a review from a team as a code owner January 27, 2025 17:43
Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

In general LGTM - minor nit on using errorf.

js.mu.RUnlock()
return false
return fmt.Errorf("stream assignment or group missing")
Copy link
Member

Choose a reason for hiding this comment

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

If not formatting used maybe errors.New()?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

This should make the output of `healthz` less opaque when there are
stream or consumer healthcheck errors.

Signed-off-by: Neil Twigg <[email protected]>
@neilalexander neilalexander force-pushed the neil/streamconsumerhealth branch from 0c7fbd3 to f3c06cb Compare January 27, 2025 19:31
@derekcollison derekcollison self-requested a review January 27, 2025 19:34
Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

LGTM

@derekcollison derekcollison merged commit 2086e62 into main Jan 27, 2025
5 checks passed
@derekcollison derekcollison deleted the neil/streamconsumerhealth branch January 27, 2025 21:44
neilalexander added a commit that referenced this pull request Feb 6, 2025
Includes the following:

- #6406
- #6412
- #6408
- #6416
- #6425
- #6424
- #6438
- #6439
- #6446
- #6447
- #6448
- #6449
- #6450
- #6451
- #6452
- #6453
- #6456
- #6458
- #6457
- #6459
- #6460
- #6461

Signed-off-by: Neil Twigg <[email protected]>
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.

2 participants