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

[ISSUE #42] fix the problem HealthExcutor don't doCheck asynchronously #43

Merged
merged 5 commits into from
Mar 1, 2024

Conversation

Lambert-Rao
Copy link
Contributor

Besides, I add an integration test for HealthService

@@ -41,8 +41,10 @@ public void update(String type, Long typeId, HealthCheckStatus status, String re
cacheMap.put(type, subMap);
}
CheckResult oldResult = subMap.get(typeId);
String oldDesc = Objects.isNull(oldResult.getResultDesc()) ? "" : oldResult.getResultDesc() + "\n";
CheckResult result = new CheckResult(status, oldDesc + resultDesc, LocalDateTime.now(),
String oldDesc = Objects.isNull(oldResult.getResultDesc()) || oldResult.getResultDesc().isEmpty() ? "" : oldResult.getResultDesc() + "\n";
Copy link
Member

Choose a reason for hiding this comment

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

Why oldDesc should have a \n at the end?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

newdesc is added after oldDesc , just to distinguish

Copy link
Member

Choose a reason for hiding this comment

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

You mean like this?

oldDesc
newdesc

By the way, Objects.isNull is intended for use within Java 8 lambda filtering. Just keep the code simple using whatever != null will be better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oldDesc
newdesc

that's what I mean

Copy link
Member

Choose a reason for hiding this comment

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

Then such writting will be better:

String whatever = oldDesc + Constants.NEW_LINE_ENDING + newDesc;

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay

Copy link
Member

Choose a reason for hiding this comment

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

Well, using a constant is meant to split oldDesc and newDesc strings. So the following is better:

String description = oldDesc + Constants.NEW_LINE_ENDING + resultDesc;

Also, a constant can be referenced by its class name to indicate its usage field, so a HEALTH_ prefix is unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if oldDesc is an empty string "", this add an NEW_LINE_ENDING is the beginning of the description.

Copy link
Member

Choose a reason for hiding this comment

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

if-else is acceptable~

@Pil0tXia Pil0tXia merged commit d816839 into apache:dev Mar 1, 2024
2 checks passed
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