Skip to content

feat: add proxy-config and activate https/http proxy for pixi commands #3320

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

Merged
merged 6 commits into from
Mar 28, 2025

Conversation

gzm55
Copy link
Contributor

@gzm55 gzm55 commented Mar 11, 2025

This pr add new config keys:

[proxy-config]
https = "http://proxy.com:8080"
http = "http://user:[email protected]"
non-proxy-hosts = [ "domain-1.org", "domain-2.org", ]

Activate the configured proxies at the entrypoint of pixi subcommands, like the https.proxy config keys for git.
With these configs, when developing multiple projects, we can setup different proxies for each projects, and do not need to write the config into pixi.toml [activation] sections.

The common proxy environments such as HTTPS_PROXY and http_proxy can overridden the whole proxy config table, so behaviors of the proxy injection via env are kept.

This pr could help the similar situations in #2757 , #1035, #474, etc.

@gzm55 gzm55 force-pushed the work/proxy-config branch from a7f8aee to 8f06129 Compare March 11, 2025 02:58
Copy link
Contributor

@baszalmstra baszalmstra left a comment

Choose a reason for hiding this comment

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

Awesome PR! However, Im a little unsure about whether setting the variables into the environment is the best solution. I would prefer to be explicit and pass them into reqwest clients where they are used instead.

@gzm55 gzm55 force-pushed the work/proxy-config branch from 8f06129 to 45c6809 Compare March 13, 2025 02:21
@gzm55 gzm55 requested a review from baszalmstra March 14, 2025 08:27
@gzm55 gzm55 mentioned this pull request Mar 18, 2025
@gzm55 gzm55 force-pushed the work/proxy-config branch from 45c6809 to 7efe89b Compare March 20, 2025 16:34
@gzm55
Copy link
Contributor Author

gzm55 commented Mar 20, 2025

@baszalmstra the proxies are now transferred into reqwest::Client, and avoid settting environment vars.

@gzm55 gzm55 force-pushed the work/proxy-config branch from 5bdfba0 to cd57f7f Compare March 21, 2025 16:25
@gzm55
Copy link
Contributor Author

gzm55 commented Mar 21, 2025

@baszalmstra proxies for uv are also injected into its inner reqwest client.

pixi run/shell/shell-hook will not inherit the proxies setttings. One needs activate script for proxies settings.

pixi and uv could also execute git fetch command to build the package from source. Because the git has its own proxy config, this pr does not inject proxies into these child processes.

@gzm55 gzm55 force-pushed the work/proxy-config branch from cd57f7f to 997e0b0 Compare March 24, 2025 12:18
@gzm55
Copy link
Contributor Author

gzm55 commented Mar 24, 2025

@baszalmstra is there other suggestion?

@baszalmstra
Copy link
Contributor

@tdejager Could you check the implementation for the uv part?
@ruben-arts Could you check the changes to the config and perhaps the general implementation?

Copy link
Contributor

@ruben-arts ruben-arts left a comment

Choose a reason for hiding this comment

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

This looks good to me!

@gzm55
Copy link
Contributor Author

gzm55 commented Mar 26, 2025

This looks good to me!

@ruben-arts The warning message was updated as you suggested.

@gzm55
Copy link
Contributor Author

gzm55 commented Mar 27, 2025

@tdejager Could you check the implementation for the uv part?

@tdejager is there some suggestion about this pr?

@tdejager
Copy link
Contributor

Looks good let's merge!

@tdejager tdejager merged commit 6c88c30 into prefix-dev:main Mar 28, 2025
29 checks passed
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.

5 participants