-
Notifications
You must be signed in to change notification settings - Fork 266
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-37184] [JENKINS-5124] Log the exception that is thrown if Jenksins can't get info() on a subversion repository using a tunnel #165
base: master
Are you sure you want to change the base?
Conversation
Log the exception that is thrown if Jenksins can't get info() on a subversion repository using a tunnel. When the repository factory creates the repository, set the tunnel provider based on default svn options.
@@ -255,6 +256,7 @@ private boolean isSVNRepositoryProjectRoot(SVNRepository repo) { | |||
return true; | |||
} | |||
} catch (SVNException e) { | |||
LOGGER.log(Level.SEVERE, "An SVN exception occurred", e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The message does not provide any value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's doesn't? It tells a sysadmin that they have a misconfigured repository line when they specify svn+foo://domain/path and foo is not a configured tunnel in .subversion/config. It's much better then throwing that exception on the floor. It's the only way I would know how to fix the bug.
Removed extra spaces.
I believe this patch is ready to be merged. If not, please provide feedback. |
Any updates on this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
merge conflicts
@@ -179,6 +179,7 @@ public DescriptorImpl getDescriptor() { | |||
SVNURL repoURL = SVNURL.parseURIDecoded(getTagsDir()); | |||
|
|||
SVNRepository repo = SVNRepositoryFactory.create(repoURL); | |||
repo.setTunnelProvider(SubversionSCM.createDefaultSVNOptions()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not about logging, is it?
Well it's been about a year, so I don't remember. I'll review and update the ticket. I can tell you, the efect of this omission is, under some conditions, the list of known braches and tags will not populate a drop down select box. I believe it was like Build Parameterized or some form like that. The root cause of the issue was not being logged. We didn't know what to fix. The feature was unusable. I don't recall what the root cause was, but I feel it was related to the fact that we use ssh+svn and our ssh uses Kerberos for authentication. |
Log the exception that is thrown if Jenksins can't get info() on a subversion repository using a tunnel. When the repository factory creates the repository, set the tunnel provider based on default svn options.