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.

Sign up for GitHub

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

Jump to bottom

[JENKINS-34819] Allow disabling the protocols individually #83

Merged
merged 1 commit into from
May 12, 2016
Merged

[JENKINS-34819] Allow disabling the protocols individually #83

merged 1 commit into from
May 12, 2016

Conversation

stephenc
Copy link
Member

  • Also ensure that a fatal unexpected error in one protocol does not prevent a fall-back

@reviewbybees

@stephenc
[Related to JENKINS-34121] Allow disabling the protocols individually
04037ec 
- Also ensure that a fatal unexpected error in one protocol does not prevent a fall-back
@ghost
Copy link

ghost commented May 12, 2016

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

oleg-nenashev
oleg-nenashev reviewed May 12, 2016
View reviewed changes
src/main/java/hudson/remoting/Engine.java
} catch (RuntimeException e) {
events.status("Protocol " + protocol.getName() + " encountered a runtime error", e);
} catch (Error e) {
events.status("Protocol " + protocol.getName() + " could not be completed due to an error", e);
Copy link
Member

@oleg-nenashev oleg-nenashev May 12, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Uhm. Do we really want to continue after it? Errors are commonly fatal

Copy link
Member Author

Choose a reason for hiding this comment

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

IOError is one that we should continue after and I fear could be one that gets thrown

Copy link
Member Author

Choose a reason for hiding this comment

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

feel free to update this PR to whatever you want, but if JnlpProtocol3 throws an Error, e.g. LinkerError, etc we should still try the others

Copy link
Member

Choose a reason for hiding this comment

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

I think it's acceptable. If the error is not handleable, it will just blow up in another place

@oleg-nenashev
Copy link
Member

Yay, remoting build failed due to the remoting bug.

Tests run: 173, Failures: 0, Errors: 0, Skipped: 0

[WARNING] Failed to notify spy hudson.maven.Maven3Builder$JenkinsEventSpy: Invalid object ID 7 iota=14
Build was aborted
Putting comment on the pull request
[WARNING] Failed to notify spy hudson.maven.Maven3Builder$JenkinsEventSpy: Invalid object ID 7 iota=14
channel stopped
[WARNING] Failed to notify spy hudson.maven.Maven3Builder$JenkinsEventSpy: java.io.IOException: Backing channel is disconnected.

@oleg-nenashev oleg-nenashev reopened this May 12, 2016
@oleg-nenashev
Copy link
Member

🐝

@oleg-nenashev oleg-nenashev merged commit 43793e5 into master May 12, 2016
@oleg-nenashev
Copy link
Member

Created a separate issue for it: https://issues.jenkins-ci.org/browse/JENKINS-34819

@oleg-nenashev oleg-nenashev changed the title [Related to JENKINS-34121] Allow disabling the protocols individually [JENKINS-34819] Allow disabling the protocols individually May 13, 2016
@oleg-nenashev oleg-nenashev mentioned this pull request May 13, 2016
Merged
oleg-nenashev added a commit to oleg-nenashev/jenkins that referenced this pull request May 17, 2016
@oleg-nenashev
[JENKINS-19445, JENKINS-34213, JENKINS-34808, JENKINS-34121] Bump rem…
09e482f 
…oting to 2.59

Changes:

2.58:
* [JENKINS-34213](https://issues.jenkins-ci.org/browse/JENKINS-34213) - Ensure that the unexporter cleans up whatever it can each sweep (jenkinsci/remoting#81)
* [JENKINS-19445](https://issues.jenkins-ci.org/browse/JENKINS-19445) Force class load on UserRequest in order to prevent deadlock on windows nodes when using JNA and Subversion (jenkinsci/remoting#82)
* [JENKINS-34808](https://issues.jenkins-ci.org/browse/JENKINS-34808) - Allow user to adjust socket timeout (jenkinsci/remoting#68)

2.59:
* [JENKINS-34819](https://issues.jenkins-ci.org/browse/JENKINS-34819) - Allow disabling the remoting protocols individually. Works around issues like [JENKINS-34121](https://issues.jenkins-ci.org/browse/JENKINS-34121) (jenkinsci/remoting#83)
@oleg-nenashev oleg-nenashev mentioned this pull request May 17, 2016
Closed
@stephenc stephenc deleted the jenkins-34121-workaround branch July 28, 2016 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@stephenc @oleg-nenashev