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

Fix #3015: Log InterruptedException in debug when it is a nominal terminating behavior #3021

Merged
merged 1 commit into from
Apr 28, 2021

Conversation

hibnico
Copy link
Contributor

@hibnico hibnico commented Apr 20, 2021

Description

Here is a review of the catch of the InterruptedException and their logs. I have done more than looking to the core code of the client. These suggestions are up for debate, and thus can be reverted in this PR.

So, every log about an nominal InterruptedException is now in debug, with the message of the exception but not the stack trace.

Special cases on some catch that I found:

  • BaseOperation#createOrReplace: there used to be a printStackTrace. I tried to improve it by a log and propagate the status of the interrupted thread. But considering the case here, maybe there is more to do.
  • CreateOrReplaceHelper#createOrReplaceItem: same considerations
  • CHEATSHEET.md: I have found these weird piece of catch without the try keyword. I took the liberty to juste remove it. Javac will raise an error about the InterruptedException to handle, I see no need to do it explicitely here in the cheat sheet.
  • the code exemples: to make things uniform, I made the logs at the warn level

A side note on PortForwardEquivalent example. I don't see how the countDownLatch is usefull there, countDown is never called on it.

Fix #3015

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Checklist

  • Code contributed by me aligns with current project license: Apache 2.0
  • I Added CHANGELOG entry regarding this change
  • I have implemented unit tests to cover my changes
  • I have added/updated the javadocs and other documentation accordingly
  • No new bugs, code smells, etc. in SonarCloud report
  • I tested my code in Kubernetes
  • I tested my code in OpenShift

@centos-ci
Copy link

Can one of the admins verify this patch?

@rohanKanojia
Copy link
Member

rohanKanojia commented Apr 20, 2021

@akram: Could you please review this PR whenever you find time, specifically informer related changes.

@akram
Copy link
Contributor

akram commented Apr 21, 2021

/LGTM

Looks good to me @hibnico , thanks for the fixes.

Copy link
Member

@manusa manusa left a comment

Choose a reason for hiding this comment

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

LGTM, thx!

@sonarcloud
Copy link

sonarcloud bot commented Apr 28, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

20.0% 20.0% Coverage
0.0% 0.0% Duplication

@manusa manusa merged commit 0282631 into fabric8io:master 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
Development

Successfully merging this pull request may close these issues.

DefaultController#processLoop interrupt should not display stackTrace
6 participants