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

CHE-3064: Add log errors on pre-destroy phase #3990

Merged
merged 3 commits into from
Feb 11, 2017
Merged

CHE-3064: Add log errors on pre-destroy phase #3990

merged 3 commits into from
Feb 11, 2017

Conversation

vinokurig
Copy link
Contributor

@vinokurig vinokurig commented Feb 1, 2017

What does this PR do?

Log the errors on pre-destroy phase

What issues does this PR fix or reference?

#3064

Changelog

Errors that might happen on application server stopping, will be logged.

Release Notes

Technical debt N/A

@codenvy-ci
Copy link

@@ -21,12 +23,8 @@
void onError(Object instance, Method method, Throwable error);

/**
* Implementation of DestroyErrorHandler that ignore errors, e.g. such behaviour is required for annotation {@link
* Implementation of DestroyErrorHandler that log errors, e.g. such behaviour is required for annotation {@link
Copy link
Contributor

Choose a reason for hiding this comment

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

actually @javax.annotation.PreDestroy does require the logging of errors and provided documentation can confused a bit.

@codenvy-ci
Copy link

Build # 1891 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/1891/ to view the results.

@vinokurig vinokurig merged commit cf01c01 into master Feb 11, 2017
@vinokurig vinokurig deleted the CHE-3064 branch February 11, 2017 08:19
@vinokurig vinokurig added this to the 5.3.0 milestone Feb 11, 2017
@JamesDrummond JamesDrummond mentioned this pull request Feb 17, 2017
9 tasks
JPinkney pushed a commit to JPinkney/che that referenced this pull request Aug 17, 2017
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.

4 participants