Fix proxy server name documentation #2060

Merged
merged 3 commits into from Feb 25, 2016

Conversation

4 participants
@olivierdagenais
Member

olivierdagenais commented Feb 24, 2016

Background

(Skip to the next section if you don't enjoy seeing a mystery being unraveled. SPOILER: the butler did it!)

I'm nearing the end of verifying proxy support for the Jenkins TFS plugin and I thought I would update the proxy documentation in the Jenkins wiki, so I could point my users to it. As I started updating the page, I decided to look up what Jenkins itself had to say about this and here's what I found:

proxyservername-before

The above screenshot contains the following text:

If your Jenkins server sits behind a firewall and does not have the direct access to the internet, and if your server JVM is not configured appropriately (See JDK networking properties for more details) to enable internet connection, you can specify the HTTP proxy server name in this field to allow Jenkins to install plugins on behalf of you. Note that Jenkins uses HTTPS to communicate with the update center to download plugins.

The value you submit here will be set as http.proxyHost system property. Leaving this field empty causes this property to be unset, which means Jenkins will try to connect to the host directly.

If you are unsure about the value, check the browser proxy configuration.

...so, naturally, I got curious about having overlooked this property during my testing and I can't find where Jenkins actually sets that property. (the only hits for http.proxyHost are in the documentation)

It looks like the file was originally added at r9915 on June 6th 2008, a.k.a. commit 7003cd8, and, at the time, there was indeed code that did this in PluginManager.java:

    public void doProxyConfigure(@QueryParameter("proxy.server") String server, @QueryParameter("proxy.port") String port, StaplerResponse rsp) throws IOException {
        System.setProperty("http.proxyHost",Util.fixEmptyAndTrim(server));
        System.setProperty("http.proxyPort",Util.fixEmptyAndTrim(port));
        rsp.sendRedirect("./advanced");
    }

...but a week later came r10078 (commit 0543909) that fixed HUDSON-1841 by no longer setting properties and instead setting the proxy field (of type ProxyConfiguration), which is still around.

Why bother?

The Jenkins Git plugin uses the Jenkins.proxy field to configure Git to use a proxy server (and has been doing so for at least two years) and soon the Jenkins TFS plugin will also do this; it would be nice to provide accurate guidance on configuring Jenkins and its plugins ONCE how to connect to a proxy server.

The actual pull request

This pull request aims to correct the documentation for the proxy server in all the cultures that reference the http.proxyHost property, so I'm going to need some help from fluent speakers/writers for those I wasn't able to do (I'm fluently French and the Turkish one was almost identical to the English version):

  • default
  • de
  • fr
  • ja
  • nl
  • pt_BR
  • tr
  • zh_TW

Should we go further?

The first paragraph of the proxy server name documentation still talks about "JDK networking properties". It seems to me we should at least deprecate that guidance. It's possible that setting those properties is still the only way to get some of the plugins to make use of the proxy server, so we may not be able to remove that guidance entirely.

@olivierdagenais

This comment has been minimized.

Show comment
Hide comment
@olivierdagenais

olivierdagenais Feb 24, 2016

Member

It looks like build 4019 failed due to hudson.model.AbstractProjectTest.testConfiguringBlockBuildWhenUpstreamBuildingRoundtrip receiving java.net.SocketTimeoutException: Read timed out while attempting to verify a page with HtmlUnit.

A cursory look at the other thread stacktraces and this test failure appears to be due to a race condition and I didn't touch any code anywhere near there, anyway.

So, what should I do? Queuing another build feels like sweeping the issue under the carpet.

Member

olivierdagenais commented Feb 24, 2016

It looks like build 4019 failed due to hudson.model.AbstractProjectTest.testConfiguringBlockBuildWhenUpstreamBuildingRoundtrip receiving java.net.SocketTimeoutException: Read timed out while attempting to verify a page with HtmlUnit.

A cursory look at the other thread stacktraces and this test failure appears to be due to a race condition and I didn't touch any code anywhere near there, anyway.

So, what should I do? Queuing another build feels like sweeping the issue under the carpet.

@daniel-beck

This comment has been minimized.

Show comment
Hide comment
@daniel-beck

daniel-beck Feb 24, 2016

Member

@olivierdagenais We don't particularly care about flaky test failures clearly unrelated to the PR, so there's no problem here.

Thanks for the doc fix!

Member

daniel-beck commented Feb 24, 2016

@olivierdagenais We don't particularly care about flaky test failures clearly unrelated to the PR, so there's no problem here.

Thanks for the doc fix!

@daniel-beck

This comment has been minimized.

Show comment
Hide comment
@daniel-beck

daniel-beck Feb 24, 2016

Member

I suspect there's something wrong with the "localized" file for the Turkish locale, so it should probably be deleted altogether.

Member

daniel-beck commented Feb 24, 2016

I suspect there's something wrong with the "localized" file for the Turkish locale, so it should probably be deleted altogether.

@olivierdagenais

This comment has been minimized.

Show comment
Hide comment
@olivierdagenais

olivierdagenais Feb 24, 2016

Member

I suspect there's something wrong with the "localized" file for the Turkish locale, so it should probably be deleted altogether.

I replaced commit f673a3b with 82ea4f9.

Member

olivierdagenais commented Feb 24, 2016

I suspect there's something wrong with the "localized" file for the Turkish locale, so it should probably be deleted altogether.

I replaced commit f673a3b with 82ea4f9.

@olivierdagenais olivierdagenais referenced this pull request in jenkinsci/tfs-plugin Feb 24, 2016

Merged

[JENKINS-6933] Handle proxy settings #72

olivergondza added a commit that referenced this pull request Feb 25, 2016

@olivergondza olivergondza merged commit b5db997 into jenkinsci:master Feb 25, 2016

1 check failed

Jenkins Looks like there's a problem with this pull request
Details
@olivierdagenais

This comment has been minimized.

Show comment
Hide comment
@olivierdagenais

olivierdagenais Feb 25, 2016

Member

Thank you for merging! As for the remaining cultures, I have traced each file's addition to determine the original author.

If you see your name here, would you mind updating core/src/main/resources/hudson/ProxyConfiguration/help-name_{culture}.html to remove mentions of the http.proxyHost property, just like I did in the English version with this pull request?

And just in case: @jenkinsci/translators

Thanks!

  • Oli
Member

olivierdagenais commented Feb 25, 2016

Thank you for merging! As for the remaining cultures, I have traced each file's addition to determine the original author.

If you see your name here, would you mind updating core/src/main/resources/hudson/ProxyConfiguration/help-name_{culture}.html to remove mentions of the http.proxyHost property, just like I did in the English version with this pull request?

And just in case: @jenkinsci/translators

Thanks!

  • Oli

@olivierdagenais olivierdagenais deleted the olivierdagenais:fix_proxy_server_name_doc branch Feb 25, 2016

russinholi added a commit to russinholi/jenkins that referenced this pull request Feb 25, 2016

daniel-beck added a commit that referenced this pull request Feb 29, 2016

@batmat

This comment has been minimized.

Show comment
Hide comment
@batmat

batmat Feb 29, 2016

Member

Ping @damianszczepanik for the polish one. Thanks!

Member

batmat commented Feb 29, 2016

Ping @damianszczepanik for the polish one. Thanks!

damianszczepanik added a commit to damianszczepanik/jenkins that referenced this pull request Mar 15, 2016

olivergondza added a commit that referenced this pull request Mar 16, 2016

Merge pull request #2064 from russinholi/master
Fix proxy server name documentation (#2060)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment