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

#47654 Adds a new `constraints` parameter to pip command #50092

Open
wants to merge 5 commits into
base: devel
from

Conversation

Projects
None yet
4 participants
@frague59
Copy link

frague59 commented Dec 18, 2018

SUMMARY

Adds the constraints argument to the pip packaging command

Fixes #47654

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

pip

ADDITIONAL INFORMATION

Adds a new parameter for the pip command: constraints

- name: Installs requirements with constraints
  pip:
    requirements: requirements.txt
    constraints: constraints.txt

GUERIN François added some commits Dec 18, 2018

GUERIN François
#47654 Adds support for constraint for pip module
+ Updates docs
+ Updates the code, `constraints` is used in conjunction with `requirements`
GUERIN François
#47654 Adds support for constraint for pip module
+ Updates docs
+ Updates the code, `constraints` is used in conjunction with `requirements`
GUERIN François
@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Dec 18, 2018

@frague59 this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Dec 18, 2018

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

lib/ansible/modules/packaging/language/pip.py:279:1: E303 too many blank lines (3)

The test ansible-test sanity --test validate-modules [explain] failed with 2 errors:

lib/ansible/modules/packaging/language/pip.py:0:0: E309 version_added for new option (constraints) should be 2.8. Currently 0.0
lib/ansible/modules/packaging/language/pip.py:9:0: E106 Import found before documentation variables. All imports must appear below DOCUMENTATION/EXAMPLES/RETURN/ANSIBLE_METADATA.

click here for bot help

@sivel
Copy link
Member

sivel left a comment

In addition to other changes listed here, please:

  1. rebase to remove the merge commits
  2. add tests for this functionality
  3. add a changelog fragment to changelog/fragments
from ansible.module_utils._text import to_native
from ansible.module_utils.basic import (AnsibleModule, is_executable,
missing_required_lib)
from ansible.module_utils.six import PY3

This comment has been minimized.

@sivel

sivel Dec 18, 2018

Member

These imports need to be moved back to where they were. As you can see moving it has caused CI to fail.

constraints:
description:
- The path to a pip constraints file, which should be local to the remote system.
File can be specified as a relative path if using the chdir option.

This comment has been minimized.

@sivel

sivel Dec 18, 2018

Member

Need version_added: "2.8" here

@@ -567,6 +585,7 @@ def main():
name=dict(type='list'),
version=dict(type='str'),
requirements=dict(type='str'),
constraints=dict(type='str'),

This comment has been minimized.

@sivel

sivel Dec 18, 2018

Member

arguments pointing to a file should use type='path'.

@@ -682,6 +702,8 @@ def main():
cmd.extend(to_native(p) for p in packages)
elif requirements:
cmd.extend(['-r', requirements])
if constraints:

This comment has been minimized.

@sivel

sivel Dec 18, 2018

Member

constraints isn't dependent on requirements. constraints can still be useful if the libraries being installed are liberal in their selection.

This should be moved outside.

module.exit_json(changed=changed, cmd=cmd, name=name, version=version,
state=state, requirements=requirements, virtualenv=env,
stdout=out, stderr=err)
if constraints:

This comment has been minimized.

@sivel

sivel Dec 18, 2018

Member

There is no reason to make this conditional. We don't do this for requirements don't do it here too. Just use a single module.exit_json

@ansibot ansibot added the stale_ci label Dec 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment