Skip to content

Conversation

vsarunas
Copy link
Contributor

@vsarunas vsarunas commented Nov 25, 2024

Read http_proxy/https_proxy/HTTP_PROXY/HTTPS_PROXY standard environment variables; and set HTTPClient() proxy configuration with one of the values.

Fixes #108

@cmcgee1024 - this adds the minimal silent use based on the environment. Most tools read one of those variables.

@cmcgee1024
Copy link
Member

cmcgee1024 commented Nov 25, 2024

@vsarunas thank you for this contribution. Overall it looks good.

Now that this functionality is in the core http client implementation, can you look at removing the ProxyHTTPRequestExecutorImpl implementation in the tests since it's doing a subset of the same things? The macOS CI will be able to verify the implementation since it uses a proxy internally.

Also, can you run the formatter on the code like this?

swift run swiftformat Sources Tests Tools Package.swift

@vsarunas
Copy link
Contributor Author

@cmcgee1024 - done!

Needed to to change HTTPRequestExecutorImpl to be a class in the same was as in the tests to be able to shutdown the httpClient

@cmcgee1024
Copy link
Member

@swift-ci test macOS

@cmcgee1024
Copy link
Member

@vsarunas unfortunately, it looks like the macOS CI is down this week and also merging of PR's is blocked until next Monday. So, this will need to wait.

In the mean time, how do you feel about writing up a short howto guide about using these environment variables for http/https proxies? We've built up a few howto guides already, published on the spi site here: https://swiftpackageindex.com/swiftlang/swiftly/main/documentation/swiftlydocs

The source code for these guides can be found in Documentation/SwiftlyDocs.docc.

@cmcgee1024
Copy link
Member

@swift-ci test macOS

@vsarunas
Copy link
Contributor Author

vsarunas commented Dec 2, 2024

@cmcgee1024 , sure! Added a small paragraph in the getting started page.

@cmcgee1024 cmcgee1024 merged commit c5518b6 into swiftlang:main Dec 2, 2024
19 checks passed
@cmcgee1024
Copy link
Member

This looks good. Thank you @vsarunas for your contribution!

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.

Add support for downloading through a proxy.

2 participants