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

DefaultController#processLoop interrupt should not display stackTrace #3015

Closed
akram opened this issue Apr 20, 2021 · 8 comments · Fixed by #3021
Closed

DefaultController#processLoop interrupt should not display stackTrace #3015

akram opened this issue Apr 20, 2021 · 8 comments · Fixed by #3021
Assignees
Milestone

Comments

@akram
Copy link
Contributor

akram commented Apr 20, 2021

When stopping an informer, the DefaultController#processLoop displays the interruption stacktrace.
This is annoying as it makes think that this is an unexpected behaviour.

It should be replaced by:

log.warn("DefaultController#processLoop got interrupted {}", t.getMessage());
@rohanKanojia
Copy link
Member

@akram : Thanks a lot for reporting this. I think it makes sense. Would you like to submit a PR for this?

@hibnico
Copy link
Contributor

hibnico commented Apr 20, 2021

I was about to open an issue about that. I even think that for a such log, it should be at the debug level.
I you agree, I am willing to make a PR about this, searching in the entire code base about catching InterruptedExceptions and their logs, and make them in debug if not. With a quick search, I even found some e.printStackTrace().

@rohanKanojia
Copy link
Member

@hibnico : I think it's here:

log.warn("DefaultController#processLoop got interrupted {}", t.getMessage(), t);

Would be awesome if you could send a PR ❤️ .

@rohanKanojia
Copy link
Member

Do you mean to refactor interrupted exceptions in informers package or across kuberenetes-client?

@hibnico
Copy link
Contributor

hibnico commented Apr 20, 2021

I am willing to do the review the entire project, I found 41 catch. But I am just suggesting to change some InterruptedException log, not every log I found. I saw some cases where an error log or a warn was justifed.

@rohanKanojia
Copy link
Member

oh, okay. Thanks a lot for clarification! Shall I assign this issue to you?

@hibnico
Copy link
Contributor

hibnico commented Apr 20, 2021

You can, no objection.

@akram
Copy link
Contributor Author

akram commented Apr 21, 2021

Hey really thank you @hibnico and @rohanKanojia , I had a branch locally with that single fix, I would have done the PR, but @hibnico seems to have done better in #3021

@manusa manusa added this to the 5.4.0 milestone Apr 28, 2021
manusa pushed a commit to hibnico/kubernetes-client that referenced this issue Apr 28, 2021
manusa pushed a commit that referenced this issue Apr 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants