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

Add Custom Proxy settings for specialize environment #16

Merged
merged 6 commits into from Mar 24, 2016

Conversation

rr-paras-patel
Copy link
Contributor

When hipchat-go client runs inside Private DC and need proxy settings to connect internet or particular development environment this settings are necessary

@@ -86,15 +86,22 @@ type ErrorResponse struct {
type Client struct {
AuthToken string
BaseURL string
ProxyURL string
Copy link
Owner

Choose a reason for hiding this comment

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

Instead of having a specified ProxyURL, let’s do this:

type Client struct {
  AuthToken string
  BaseURL string
  Timeout  time.Duration
  Transport http.RoundTripper
}

Then instead of using http.Get, you create a client using these properties:

client := &http.Client{
  Transport: c.Transport,
  Timeout:   c.Timeout,
}

and make the request that way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can you be more specific about your comment ? do you want to remove duplicate code and implement single client for all methods (PostMessage, ListRoom) ? or do you wann specify proxy timeout and round-tripper settings ?

Copy link
Owner

Choose a reason for hiding this comment

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

  • Add the Timeout and Transport properties to the Client struct.
  • In each instance where http.Get or http.PostForm is used, do something like this:
    req, err := http.NewRequest(...)
    if err != nil {
        // Handle error
    }
    // Create the HTTP client
    client := &http.Client{
        Transport: c.Transport,
        Timeout:   c.Timeout,
    }

    // Make the request
    res, err := client.Do(req)
    if err != nil {
        // Handle error
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have implemented that http.newRequest and client.do for all GET, POST now only one thing i need to ask is
where do we ask for proxy settings from user ? may i make method e.g.

c.Transport := hipchat.SetProxy{"<PROXY_URL>", "<PROXY_PORT>"} 
or handle with 
c := hipchat.NewProxyClient("<AUTH TOKEN>","<PROXY_URL:PROXY_PORT>")

sorry for too many questions i am new to GoLang.

Copy link
Owner

Choose a reason for hiding this comment

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

Proxy settings can now be set via setting the Transport property after creating the client. This library shouldn’t care about proxies specifically.

c := hipchat.NewClient("<PUT YOUR AUTH TOKEN HERE>")
c.Transport = &http.Transport{Proxy: http.ProxyURL("<PROXY URL>")}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

noted 👍

}

// NewClient allocates and returns a Client with the given authToken.
// By default, the client will use the publicly available HipChat servers.
// For internal or custom servers, set the BaseURL field of the Client.
func NewClient(authToken string) Client {
return Client{AuthToken: authToken, BaseURL: defaultBaseURL}
return Client{AuthToken: authToken, BaseURL: defaultBaseURL, Transport: http.DefaultTransport}
Copy link
Owner

Choose a reason for hiding this comment

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

place each property on its own line

return Client {
  AuthToken: authToken,
  ...
}

Copy link
Owner

Choose a reason for hiding this comment

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

Still need to do this.


proxyURL, err := url.Parse("<PROXY_URL:PROXY_PORT>")
if err != nil {
log.Printf("Expected no error, but got %q", err)
Copy link
Owner

Choose a reason for hiding this comment

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

log.Fatalf

@rr-paras-patel
Copy link
Contributor Author

@andybons so what are we waiting for ?

@andybons andybons merged commit c9ecf9b into andybons:master Mar 24, 2016
@rr-paras-patel rr-paras-patel deleted the Paras/Set_Custome_proxy branch April 20, 2016 05:50
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.

None yet

2 participants