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

chore: More detailed logging for ignored handling #20007

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

caalador
Copy link
Contributor

Make log more detailed for
ignored invocation handle.

Make log more detailed for
ignored invocation handle.
Copy link

github-actions bot commented Sep 20, 2024

Test Results

1 137 files  ± 0  1 137 suites  ±0   1h 9m 37s ⏱️ +17s
7 399 tests ± 0  7 349 ✅ ± 0  50 💤 ±0  0 ❌ ±0 
7 771 runs  +20  7 711 ✅ +20  60 💤 ±0  0 ❌ ±0 

Results for commit 0d3dffe. ± Comparison against base commit 2e28faf.

♻️ This comment has been updated with latest results.

@@ -723,4 +724,14 @@ public static Router getRouter(HasElement component) {
return router;
}

public static Optional<Component> getRouteComponent(Component component) {
Copy link
Member

Choose a reason for hiding this comment

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

JavaDoc missing from a public API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe not for this PR, but we have a findAncestor(Class) method in Component class.
It could be useful to have also a findAncestor(Predicate<Component>) for similar cases

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps if there comes up a need for this.

getLogger().info(
"Ignored RPC for invocation handler '{}' from "
+ "the client side for an {} node id='{}' {}",
getClass().getName(), reason, node.getId(), targetInfo);
Copy link
Member

Choose a reason for hiding this comment

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

Both if-else blocks ends with mostly duplicate lines of code so it can be moved out side the if-else blocks in the end of the method and targetInfo could be blank by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

unify log.
Copy link

sonarcloud bot commented Sep 20, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants