Private/babbara/enable proxy setting #17
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,25 +1,38 @@ | ||
| package clients | ||
|
|
||
| // HTTPMaxRetry indicates the number of | ||
| // retries to be carried out before giving up. | ||
| const HTTPMaxRetry = 9 | ||
|
|
||
| // Client struct encapsulate the collection of | ||
| // external services | ||
| type Client struct { | ||
| Resmgr Resmgr | ||
| Keystone Keystone | ||
| Qbert Qbert | ||
| Executor Executor | ||
| Segment Segment | ||
| HTTP HTTP | ||
| } | ||
|
|
||
| // New creates the clients needed by the CLI | ||
| // to interact with the external services. | ||
| func New(fqdn string, proxy string) (Client, error) { | ||
|
|
||
| http, err := NewHTTP( | ||
| func(impl *HTTPImpl) { impl.Proxy = proxy }, | ||
|
There was a problem hiding this comment. Why are we passing in functions ? Why don't we just pass in the value ? |
||
| func(impl *HTTPImpl) { impl.Retry = HTTPMaxRetry }) | ||
|
|
||
| if err != nil { | ||
| return Client{}, err | ||
| } | ||
|
|
||
| return Client{ | ||
| Resmgr: NewResmgr(fqdn, http), | ||
| Keystone: NewKeystone(fqdn, http), | ||
| Qbert: NewQbert(fqdn, http), | ||
| Executor: ExecutorImpl{}, | ||
| Segment: NewSegment(fqdn), | ||
|
There was a problem hiding this comment. I would like to understand more on how we have used in different places, in general I would like to see more "events" and segment should be able to "consume" those events instead of directly using segment. |
||
| HTTP: http, | ||
| }, nil | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| package clients | ||
|
|
||
| import ( | ||
| "net/http" | ||
| "net/url" | ||
| "time" | ||
|
|
||
| "github.com/PuerkitoBio/rehttp" | ||
| ) | ||
|
|
||
| type HTTP interface { | ||
| Do(req *http.Request) (*http.Response, error) | ||
| } | ||
|
|
||
| type HTTPImpl struct { | ||
| Proxy string | ||
| Retry int | ||
| client *http.Client | ||
| ProxyURL *url.URL | ||
| } | ||
|
|
||
| func NewHTTP(options ...func(*HTTPImpl)) (*HTTPImpl, error) { | ||
| resp := &HTTPImpl{} | ||
|
|
||
| for _, option := range options { | ||
| option(resp) | ||
| } | ||
|
|
||
| var transport http.RoundTripper | ||
| if resp.Proxy != "" { | ||
| proxyURL, err := url.Parse(resp.Proxy) | ||
|
There was a problem hiding this comment. We should parse it when user enters the proxy and error out. ProxyURL can be used inside the code instead of passing proxy as string There was a problem hiding this comment. Fail at the input site only you mean ?! |
||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| resp.ProxyURL = proxyURL | ||
| transport = &http.Transport{Proxy: http.ProxyURL(proxyURL)} | ||
| } else { | ||
| transport = http.DefaultTransport | ||
| } | ||
|
|
||
| t := rehttp.NewTransport(transport, rehttp.RetryAll( | ||
| rehttp.RetryAny( | ||
| rehttp.RetryTemporaryErr(), | ||
| rehttp.RetryStatuses(400, 404), | ||
|
There was a problem hiding this comment. What about when gateway times out, I think it is 502 or 503 I can't recall There was a problem hiding this comment.
|
||
| ), | ||
| rehttp.RetryMaxRetries(resp.Retry), | ||
| ), | ||
| // rehttp.ExpJitterDelay(time.Second*time.Duration(2), time.Second*time.Duration(60)) | ||
| rehttp.ConstDelay(time.Second*time.Duration(10))) | ||
|
|
||
| resp.client = &http.Client{Transport: t} | ||
| return resp, nil | ||
| } | ||
|
|
||
| // Do function simply calls the underlying client to make the request. | ||
| func (c HTTPImpl) Do(req *http.Request) (*http.Response, error) { | ||
| return c.client.Do(req) | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I would call it an HTTPClient