Skip to content
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

Add support for user and constraints to pip module #33098

Closed
wants to merge 1 commit into from

Conversation

emonty
Copy link
Contributor

@emonty emonty commented Nov 20, 2017

SUMMARY

pip supports a --user flag, which tells pip to install to the Python
user install directory. Add it as a boolean flag.

pip also supports a -c/--constraint option that can be used to supply a
file containing constraints to apply to how pip resolves dependencies.
Add a parameter allowing a user to provide a path to a constraints file.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

lib/ansible/modules/packaging/language/pip.py

ADDITIONAL INFORMATION

Both of these can be put into extra_args of course, but it doing so in playbooks where they are optional parameters starts to get really awkward, while having them as parameters allows use of things like default(omit)

@ansibot
Copy link
Contributor

ansibot commented Nov 20, 2017

@ansibot ansibot added affects_2.5 This issue/PR affects Ansible v2.5 core_review In order to be merged, this PR must follow the core review workflow. feature_pull_request module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests. labels Nov 20, 2017
@ansibot
Copy link
Contributor

ansibot commented Nov 20, 2017

The test ansible-test sanity --test validate-modules [?] failed with the following error:

lib/ansible/modules/packaging/language/pip.py:0:0: E309 version_added for new option (constraints) should be 2.5. Currently 0.0

click here for bot help

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Nov 20, 2017
@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Nov 20, 2017
@mattclay
Copy link
Member

CI failure in integration tests:

{
    "assertion": "'Successfully installed iso8601-0.1.11' in req_installed.stdout_lines", 
    "changed": false, 
    "evaluated_to": false
}

@mattclay mattclay added the ci_verified Changes made in this PR are causing tests to fail. label Nov 21, 2017
openstack-gerrit pushed a commit to openstack-infra/zuul-jobs that referenced this pull request Nov 21, 2017
Sphinx jobs and reno jobs need basically the same thing for
dependencies. So make a new role, ensure-sphinx, which is
parameterizable enough that it can be used by both reno and sphinx jobs.

Make build jobs for both releasenotes and sphinx docs, as both of these
things are perfectly valid things to do in both OpenStack and
non-OpenStack contexts. We'll add an openstack specific job in
openstack-zuul-jobs that uses these as parents but adds the requirements
repo and constraints file settings.

Some of the pip commands here can be improved once
ansible/ansible#33098 lands and is released,
which would allow specifying --user and -c as parameters to the pip
module.

Change-Id: Idd7caf7d88b56d61872906027b4ce7d743572ded
Needed-By: I57de14580f39b9e1c11a587b51b44b61b02c84da
Copy link
Contributor

@robinro robinro 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 this PR.

It looks good overall and we should be able to merge it, once the tests are green.

# Once it does/can, change this to verify using req_installed.version instead
# of looking in stdout_lines
- name: check that version of package was installed that we expect
assert:
Copy link
Contributor

Choose a reason for hiding this comment

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

This check should be after the register task.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Heh. Duh. Thanks.

user: yes

# TODO pip module does not report back version in the return info.
# Once it does/can, change this to verify using req_installed.version instead
Copy link
Contributor

Choose a reason for hiding this comment

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

Feel free to implement this :)
It shouldn't be too hard. We already return the version when given as input and the version is available in _is_present.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool - I'll do that in a second PR.

@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 Nov 30, 2017
@ansibot ansibot removed ci_verified Changes made in this PR are causing tests to fail. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Nov 30, 2017
pip supports a --user flag, which tells pip to install to the Python
user install directory. Add it as a boolean flag.

pip also supports a -c/--constraint option that can be used to supply a
file containing constraints to apply to how pip resolves dependencies.
Add a parameter allowing a user to provide a path to a constraints file.
@mattclay
Copy link
Member

mattclay commented Dec 1, 2017

CI failure in integration tests. Most of the errors appear to be: failure: :stderr: Usage: pip uninstall [options] <package> ... pip uninstall [options] -r <requirements file> ... no such option: --user

The RHEL 7.4 failure is unrelated to the PR and should be resolved when CI runs again.

@mattclay mattclay added the ci_verified Changes made in this PR are causing tests to fail. label Dec 1, 2017
@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 Dec 9, 2017
@robinro
Copy link
Contributor

robinro commented Dec 10, 2017

@emonty The tests are still not green. Can you have another look?
You can run the tests locally via e.g.
source hacking/env-setup; ansible-test integration --docker=ubuntu1604 pip

@ansibot ansibot added feature This issue/PR relates to a feature request. and removed feature_pull_request labels Mar 2, 2018
@ansibot
Copy link
Contributor

ansibot commented Nov 10, 2018

@ansibot ansibot added the packaging Packaging category label Feb 17, 2019
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label May 16, 2020
@emonty emonty closed this May 20, 2020
@ansible ansible locked and limited conversation to collaborators Jun 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.5 This issue/PR affects Ansible v2.5 ci_verified Changes made in this PR are causing tests to fail. feature This issue/PR relates to a feature request. module This issue/PR relates to a module. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. packaging Packaging category stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants