Skip to content

ansible-test - add proxy support to pip #81117

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

Open
wants to merge 4 commits into
base: devel
Choose a base branch
from

Conversation

chrros95
Copy link

Add proxy support to pip

SUMMARY

With this commit the --proxy parameter is added to pip the pip options if the command is install.
The value is sourced from the os environment. As only one proxy can be used by pip the first matching environment variable that exists will be used. Nowadys most sources are transfered vis https so proxies with HTTPS are prefered. As there is no convention weather the variables are lower or upper case - both are checked.

The no_proxy vars are already accepted as pip currently prepares to support a no_proxy option (pypa/pip#5378)

fixes #77304

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

ansible-test (sanity)

chrros95 added 2 commits June 23, 2023 18:06
With this commit the --proxy parameter is added to pip install.
The value is sourced from the os environment. fixes ansible#77304
@ansibot ansibot added WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. affects_2.16 bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. test This PR relates to tests. labels Jun 23, 2023
@ansibot
Copy link
Contributor

ansibot commented Jun 23, 2023

The test ansible-test sanity --test pylint [explain] failed with 1 error:

test/lib/ansible_test/_util/target/setup/requirements.py:193:23: invalid-name: Argument name "isInstall" doesn't conform to snake_case naming style

click here for bot help

@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. and removed ci_verified Changes made in this PR are causing tests to fail. labels Jun 23, 2023
@chrros95 chrros95 marked this pull request as ready for review June 23, 2023 19:36
@ansibot ansibot removed the WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. label Jun 23, 2023
@mattclay mattclay changed the title fix: added proxy support to pip ansible-test - add proxy support to pip Jun 23, 2023
@mattclay mattclay removed the needs_triage Needs a first human triage before being processed. label Jun 23, 2023
@mattclay mattclay self-requested a review June 23, 2023 23:05
Copy link
Member

@mattclay mattclay left a comment

Choose a reason for hiding this comment

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

This PR doesn't really address #77304 -- that issue is about lack of proxy support for bootstrapping pip. The current implementation in this PR doesn't support delegation, such as with the --docker or --remote options.

@chrros95 Can you explain more about your use case? Is there a reason you can't use a pip configuration file instead?

@chrros95 chrros95 requested a review from a team as a code owner June 24, 2023 11:43
@chrros95
Copy link
Author

Thanks for the review.

Adding the --proxy option doesn't solve the bootstrapping issue. The issue is solved by the extended allowlist in test/lib/ansible_test/_internal/util.py. With this extension *_proxy vars are not filtered and urllib can detect a proxy to download the get-pip file.

I have added a commit so that the *_proxy vars are passed to the containers. For the --remote option. For the --remote option I don't have a clue what to change and what might be useful. The system might be in a different location and needs to use a different/no proxy. Furthermore, the sys admin should be able to provide the *_proxy vars on its own.

My project run inside a docker container which is regularly destroyed and where I'm not allowed to modify any startup parameters. To avoid a regular manual creation of the pip configuration file, I would appreciate it if it works out "of the box".

@ansibot ansibot added docs This issue/PR relates to or includes documentation. docsite This issue/PR relates to the documentation website. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jun 24, 2023
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jul 4, 2023
@ansibot ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_review Updates were made after the last review and the last review is more than 7 days old. has_issue labels Jul 12, 2023
@ansibot ansibot removed the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Oct 24, 2023
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Oct 26, 2023
@Akasurde
Copy link
Member

cc @mattclay

@ansibot ansibot added the stale_pr This PR has not been pushed to for more than one year. label Jan 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.16 bug This issue/PR relates to a bug. docs This issue/PR relates to or includes documentation. docsite This issue/PR relates to the documentation website. has_issue needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html new_contributor This PR is the first contribution by a new community member. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. stale_pr This PR has not been pushed to for more than one year. stale_review Updates were made after the last review and the last review is more than 7 days old. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot run ansible-test sanity behind a proxy
4 participants