Skip to content

Conversation

alexkayabula
Copy link
Contributor

@alexkayabula alexkayabula commented Jun 17, 2020

What does this PR do?

  • This PR provides fixes #553.

Description of task to be completed?

  • The change provides the ability to set a proxy from specified environment variables i.e HTTP_PROXY, HTTPS_PROXY, NO_PROXY

Copy link
Member

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

@alexkayabula alexkayabula force-pushed the enable-proxy-setting branch 2 times, most recently from 0cd38da to 20ea4c6 Compare June 18, 2020 11:32
@alexkayabula alexkayabula requested a review from brikis98 June 18, 2020 11:44
@alexkayabula alexkayabula force-pushed the enable-proxy-setting branch 13 times, most recently from fb46d77 to c31c413 Compare June 19, 2020 10:50
@@ -31,9 +31,18 @@ func HttpGet(t testing.TestingT, url string, tlsConfig *tls.Config) (int, string
func HttpGetE(t testing.TestingT, url string, tlsConfig *tls.Config) (int, string, error) {
logger.Logf(t, "Making an HTTP GET call to URL %s", url)

// Get a deepcopy of the default Transport implementation
DefaultTransporter := http.DefaultTransport.(*http.Transport).Clone()
Copy link
Member

Choose a reason for hiding this comment

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

If Clone does a deep copy, couldn't this be simplified to:

tr := http.DefaultTransport.(*http.Transport).Clone()

That would make the code more maintainable too in case http.Transport adds new fields in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for pointing that out.

@alexkayabula alexkayabula force-pushed the enable-proxy-setting branch 5 times, most recently from b29a604 to 815ac0d Compare June 22, 2020 09:44
@alexkayabula alexkayabula force-pushed the enable-proxy-setting branch from 815ac0d to bd9079b Compare June 22, 2020 10:24
@alexkayabula alexkayabula requested a review from brikis98 June 22, 2020 13:06
Copy link
Member

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

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

Thank you! I'll kick off tests now.

@brikis98
Copy link
Member

Tests passed! Merging now.

@brikis98 brikis98 merged commit 4c6cc0d into gruntwork-io:master Jun 23, 2020
@brikis98
Copy link
Member

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.

http_helper.HttpGetWithRetry does not use global proxy settings
2 participants