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

Change logResult to only log once #229

Merged
merged 2 commits into from
Mar 9, 2021

Conversation

mtyazici
Copy link
Contributor

@mtyazici mtyazici commented Mar 4, 2021

This fixes the issue #222

However, there is a problem to consider. The cause of this log output is empty address list. This is not mutually exclusive with the errors NoCredentialsException and RestClientException. If address list is empty and one of the exceptions is thrown, user only will be notified by the empty address list. Upon fixing the empty address issue, they will see the other exceptions. We could solve this issue by using either an integer or an array instead of a boolean but I am not sure if the case I mentioned is a real life problem.

@mtyazici mtyazici marked this pull request as draft March 4, 2021 18:13
@leszko
Copy link

leszko commented Mar 8, 2021

This fixes the issue #222

However, there is a problem to consider. The cause of this log output is empty address list. This is not mutually exclusive with the errors NoCredentialsException and RestClientException. If address list is empty and one of the exceptions is thrown, user only will be notified by the empty address list. Upon fixing the empty address issue, they will see the other exceptions. We could solve this issue by using either an integer or an array instead of a boolean but I am not sure if the case I mentioned is a real life problem.

@mtyazici I think that the "emptyList" needs a separate bool variable. Because we may need to see both the exception logs and the empty list log.

@mtyazici
Copy link
Contributor Author

mtyazici commented Mar 8, 2021

This fixes the issue #222
However, there is a problem to consider. The cause of this log output is empty address list. This is not mutually exclusive with the errors NoCredentialsException and RestClientException. If address list is empty and one of the exceptions is thrown, user only will be notified by the empty address list. Upon fixing the empty address issue, they will see the other exceptions. We could solve this issue by using either an integer or an array instead of a boolean but I am not sure if the case I mentioned is a real life problem.

@mtyazici I think that the "emptyList" needs a separate bool variable. Because we may need to see both the exception logs and the empty list log.

Sure, I will change it .

@mtyazici mtyazici marked this pull request as ready for review March 8, 2021 10:09
Copy link

@leszko leszko left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Please wait for the review from @alparslanavci and then feel free to merge.

@mtyazici mtyazici merged commit 88c367d into hazelcast:master Mar 9, 2021
@mtyazici mtyazici deleted the fix-repeating-logs branch March 9, 2021 12:03
@hasancelik hasancelik added this to the 3.3.1 milestone Mar 9, 2021
@mtyazici mtyazici linked an issue Mar 9, 2021 that may be closed by this pull request
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.

Standalone start warning is logged many times.
4 participants