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

Use custom HttpConnector when authentication #120

Closed
wants to merge 1 commit into from

Conversation

ohtake
Copy link
Contributor

@ohtake ohtake commented Sep 3, 2014

I'm trying to modify ghprb-plugin so that it can work behind authentication proxies.
I added a HttpConnector implementation to ghprb-plugin:

public class HttpConnectorWithJenkinsProxy implements HttpConnector{
    public HttpURLConnection connect(URL url) throws IOException {
        return (HttpURLConnection)ProxyConfiguration.open(url);
    }
}

Since all GitHub constructors are private, there is no way to call #setConnector before GitHub(String, String, Sttring, String) constructor calls #getMyself, nor to extend GitHub class.
So I added HttpConnector parameter to connect* methods, except connectToEnterprise which does not require proxies in most cases.

@buildhive
Copy link

Kohsuke Kawaguchi » github-api #221 SUCCESS
This pull request looks good
(what's this?)

@ohtake
Copy link
Contributor Author

ohtake commented Sep 3, 2014

A lot of overloads of connect* are hard to use.
I think it is better to introduce a builder class.

public class GitHubBuilder {
    public GitHubBuilder() {}

    public static GitHubBuilder fromPropertyFile() {}

    public GitHubBuilder withEndpoint(String endpoint) {}
    public GitHubBuilder withPassword(String user, String password) {}
    public GitHubBuilder withOAuthToken(String token) {}
    public GitHubBuilder withHttpConnector(HttpConnector connector) {}

    public GitHub build() {}
}

I close this pull request and will create another one tomorrow.

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.

2 participants