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

Plugin respects Jenkins proxy settings #42

Merged
merged 6 commits into from Aug 5, 2018

Conversation

Projects
None yet
7 participants
@ml-extern
Copy link

commented Mar 6, 2017

With these additions, the plugin will respect the proxy settings in Jenkins and configure the http connection accordingly.

This allows the user to address servers both within and outside the local area network and still use this plugin.

@tbores

This comment has been minimized.

Copy link

commented Mar 7, 2017

We would also need this change. When could be the change merged?

@@ -169,10 +190,21 @@ private HttpClient createPreconfiguredHttpClient() {
httpclient.getParams().setParameter(ConnRoutePNames.DEFAULT_PROXY, proxy);
}

/// Logging output ////////////////

This comment has been minimized.

Copy link
@Brantone

Brantone Mar 7, 2017

Collaborator

The white space formatting here is inconsistent with rest of the file / project. Please clean it up.

}

// Proxy setting, we have a Proxy _and_ the provided URL does not match any no-proxy-override
if (hasProxy &&

This comment has been minimized.

Copy link
@Brantone

Brantone Mar 7, 2017

Collaborator

Why split onto 2 lines?

@dannascimento

This comment has been minimized.

Copy link

commented Aug 20, 2017

We would also need this change. When could be the change merged?

@jjmiv

This comment has been minimized.

Copy link

commented Jul 12, 2018

is this related to the java socket timeouts and requiring a proxy login?

what's the status of this getting merged? let me know if i can assist.

@mezpahlan

This comment has been minimized.

Copy link

commented Jul 17, 2018

@jjmiv Apologies for the late reply. I became a maintainer well after this PR was raised. As best as I can tell it's to respect a proxy if it is set and to better handle edge cases related to various proxy settings.

The automatic checks tell me there are conflicts that need resolving. I'll have a look at those on the weekend and see about getting this merged.

Thanks for the offer of help. I'll reach out if I need any assistance.

p.s. I think there was a very similar PR merged in the last release that also aimed to resolve proxy issues. If so that may supercede this one. I'll find out. Could be we need both?

@showpony

This comment has been minimized.

Copy link

commented Aug 3, 2018

I've been having trouble using this jenkins plugin to upload builds to hockey - I'm getting java.net.SocketTimeoutException: Read timed out errors. I think it has to do with my company's proxy setup- Hopefully this PR will fix that! crossing fingers

@mezpahlan

This comment has been minimized.

Copy link

commented Aug 4, 2018

Hi @jjmiv, @showpony apologies both had a tough couple weeks at work not really had time to look back into this. Thanks for the offer of help. Here's the status as I see it.

At present, the PR is failing some checks. Probably because the master branch has moved on quite a bit since this was raised. I'll see what I can do to update this PR and bring it into line.

Company proxy set ups! Tell me about it! This may help but just in case it doesn't can you list here what exactly the issue is with your company proxy set up. Could need a separate bug raised for the plugin or (best case) this resolves the issue.

@mezpahlan

This comment has been minimized.

Copy link

commented Aug 4, 2018

Hmm...... seems in a wacky state. I tried a simple conflict resolution that got further than before but the check on the server are still failing.

mezpahlan added some commits Aug 4, 2018

Use a proxy configuration only if needed
The Jenkins configuration may optionally define a list of no proxy hosts
 in which case we should not attempt to use a proxied connection.
@mezpahlan

This comment has been minimized.

Copy link

commented Aug 5, 2018

OK, this passes the server checks. Turned out to be a findbugs error but wasn't immediately clear in the logs on the build server. Happy to merge this in now.

Although if anyone is reading we're putting a lot of responsibilities in a single class. I'd appreciate some help refactoring this project.

@mezpahlan mezpahlan merged commit 359a492 into jenkinsci:master Aug 5, 2018

1 check passed

continuous-integration/jenkins/pr-merge This commit looks good
Details

mezpahlan added a commit to mezpahlan/hockeyapp-plugin that referenced this pull request Sep 25, 2018

mezpahlan added a commit that referenced this pull request Sep 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.