JAMES-4068 Have several health checks in one rest calls#2399
Merged
Arsnael merged 6 commits intoSep 9, 2024
Merged
Conversation
Member
quantranhong1999
left a comment
quantranhong1999
left a comment
There was a problem hiding this comment.
webadmin documentation needed
Comment on lines
+63
to
+71
| private final Set<HealthCheck> healthChecks; | ||
| private final Map<ComponentName, HealthCheck> healthCheckMap; |
Member
There was a problem hiding this comment.
How about keep using Set<HealthCheck> healthChecks;. We still can filter valid component names because the HealthCheck has ComponentName in itself. And remove the ugly and cheating reflection below.
Contributor
Author
There was a problem hiding this comment.
"We still can filter valid component names because the HealthCheck has ComponentName in itself." ----> with Set healthChecks, iterating through the set is needed and the cost is perf.
Member
There was a problem hiding this comment.
iterating through the set is needed and the cost is perf.
Not that much IMO:
- the set size is small
- healthcheck does not need to be super performant
Contributor
Author
There was a problem hiding this comment.
discussed with @quantranhong1999 --> ignore
Contributor
There was a problem hiding this comment.
Arsnael
reviewed
Sep 5, 2024
vttranlina
reviewed
Sep 5, 2024
vttranlina
reviewed
Sep 5, 2024
chibenwa
requested changes
Sep 5, 2024
Comment on lines
+63
to
+71
| private final Set<HealthCheck> healthChecks; | ||
| private final Map<ComponentName, HealthCheck> healthCheckMap; |
Contributor
There was a problem hiding this comment.
chibenwa
reviewed
Sep 6, 2024
chibenwa
reviewed
Sep 6, 2024
chibenwa
reviewed
Sep 6, 2024
| public Object validateHealthChecks(Request request, Response response) { | ||
| Set<ComponentName> selectedComponentNames = | ||
| Arrays.stream(Optional.ofNullable(request.queryParamsValues(QUERY_PARAM_COMPONENT_NAMES)).orElse(new String[0])) | ||
| Optional.ofNullable(request.queryParamsValues(QUERY_PARAM_COMPONENT_NAMES)).map(Stream::of).orElse(Stream.empty()) |
Contributor
There was a problem hiding this comment.
Maybe we can be lucky enough to have good looking indent / method extraction / ???
Arsnael
reviewed
Sep 6, 2024
quantranhong1999
approved these changes
Sep 9, 2024
Arsnael
approved these changes
Sep 9, 2024
chibenwa
approved these changes
Sep 9, 2024
samiulsami
pushed a commit
to opnpulse/james-project
that referenced
this pull request
Feb 12, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.