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

[JENKINS-35190] Do not invoke PingFailureAnalyzer for agent=>master ping failures #2377

Merged
merged 1 commit into from
May 29, 2016

Conversation

jglick
Copy link
Member

@jglick jglick commented May 27, 2016

Related to jenkinsci/remoting#85.

@reviewbybees

@stephenc
Copy link
Member

🐝

/** Keep in a separate method so we do not even try to do class loading on {@link PingFailureAnalyzer} from an agent JVM. */
private void analyze(Throwable cause) throws IOException {
for (PingFailureAnalyzer pfa : PingFailureAnalyzer.all()) {
pfa.onPingFailure(channel,cause);
Copy link
Member

Choose a reason for hiding this comment

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

If you modify this code, please add handling of Runtime Exceptions

Copy link
Member Author

Choose a reason for hiding this comment

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

It would be nice, though (a) there are no implementations to test, as noted in the description, (b) I spent some time testing the patch as written and would have to spend more time retesting with any other modifications.

@oleg-nenashev
Copy link
Member

🐝 with a comment related to the legacy code

@oleg-nenashev oleg-nenashev changed the title [JENKINS-35190] PingThread fixes [JENKINS-35190] Do not invoke PingFailureAnalyzer for agent=>master ping failures May 29, 2016
@oleg-nenashev
Copy link
Member

We also have +1 from Oliver, so I'm going to merge it . #2341 will be integrated manually since it makes sense in any case

@oleg-nenashev oleg-nenashev merged commit 0ae6c42 into jenkinsci:master May 29, 2016
@jglick jglick deleted the PingThread-JENKINS-35190 branch May 31, 2016 18:14
samatdav pushed a commit to samatdav/jenkins that referenced this pull request Jul 25, 2016
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.

3 participants