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.

Sign up for GitHub

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

Jump to bottom

Allow arbitrary arguments to be passed to pip through pip.installed #52327

Merged
merged 15 commits into from
Apr 22, 2019
Merged

Allow arbitrary arguments to be passed to pip through pip.installed #52327

merged 15 commits into from
Apr 22, 2019

Conversation

Akm0d
Copy link
Contributor

@Akm0d Akm0d commented Mar 26, 2019

What does this PR do?

Makes it possible to pass arbitrary arguments to pip from pip.installed

What issues does this PR fix or reference?

#24751

Previous Behavior

N/A

New Behavior

pip keyword and positional arguments not yet implemented in salt can be passed to pip through pip.installed

.. code-block:: yaml

    pandas:
      pip.installed:
        - name: pandas
        - pip_future:
          - --latest-pip-kwarg:
            - param1
            - param2
          - --latest-pip-arg

Will be translated into the following pip command:

.. code-block:: bash

    pip install pandas --latest-pip-kwarg param1 --latest-pip-kwarg parm2 --latest-pip-arg

Tests written?

Yes

Commits signed with GPG?

Yes

@Akm0d Akm0d requested a review from a team as a code owner March 26, 2019 17:38
@Akm0d Akm0d changed the title Fix pip Allow arbitrary arguments to be passed to pip through pip.installed Mar 26, 2019
@waynew
Copy link
Contributor

waynew commented Mar 26, 2019

Uh... I think you might want to rebase your fixes :trollface: (or retarget your PR)

@twangboy
Copy link
Contributor

@akmod Looks like you have some lint

@twangboy
Copy link
Contributor

https://jenkinsci.saltstack.com/job/pr-lint/job/PR-52327/1/warnings52Result/new/

@Akm0d
Copy link
Contributor Author

Akm0d commented Mar 27, 2019

Failing on Centos7 with python3 and windows, but not because of the commit. Centos7 and Windows are having issues in Jenkins

@waynew
Copy link
Contributor

waynew commented Mar 28, 2019

I'm not sure if the remaining problem is legit something with these tests - it is a problem in the pip state tests, so probably worth looking into 👍

@Akm0d Akm0d requested a review from waynew April 3, 2019 00:07
@dwoz dwoz requested a review from thatch45 April 3, 2019 16:33
waynew
waynew approved these changes Apr 3, 2019
View reviewed changes
Copy link
Contributor

@waynew waynew left a comment

Choose a reason for hiding this comment

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

👍

@Akm0d
Merge branch 'develop' into fix_pip
4dc258b
@Akm0d
Renamed pip_future and added documentation to pip_state
0bb495d 
- Changed `pip_future` to `extra_args`
- Changed the execution module code block to a cli example
- Moved the state.sls example to pip_state.py
@Akm0d
Merge branch 'develop' of github.com:saltstack/salt into fix_pip
0b0b28b
@Akm0d
Changed pip_future to extra_args in test cases
8886406 
- solves saltstack#24751
twangboy
twangboy requested changes Apr 18, 2019
View reviewed changes
Copy link
Contributor

@twangboy twangboy left a comment

Choose a reason for hiding this comment

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

A few minor things. Looks great.

salt/modules/pip.py Show resolved Hide resolved
salt/modules/pip.py Show resolved Hide resolved
salt/states/pip_state.py Show resolved Hide resolved
@Akm0d Akm0d requested a review from twangboy April 22, 2019 20:23
@Akm0d Akm0d merged commit 3593c91 into saltstack:develop Apr 22, 2019
@Akm0d Akm0d deleted the fix_pip branch April 22, 2019 21:02
@max-arnold
Copy link
Contributor

Can someone backport this to master branch?

@max-arnold max-arnold mentioned this pull request Nov 11, 2019
Merged
@Akm0d Akm0d added this to the Approved milestone Dec 2, 2019
@Akm0d Akm0d mentioned this pull request Dec 2, 2019
Merged
@Akm0d
Copy link
Contributor Author

Akm0d commented Dec 2, 2019

@max-arnold Done 👍

@garethgreenaway garethgreenaway added has master-port port to master has been created and removed master-port labels Mar 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thatch45 thatch45 thatch45 approved these changes

@waynew waynew waynew approved these changes

@twangboy twangboy Awaiting requested review from twangboy

Assignees
No one assigned
Labels
has master-port port to master has been created
Projects
None yet
Milestone
Approved
Development

Successfully merging this pull request may close these issues.
6 participants
@Akm0d @waynew @twangboy @max-arnold @thatch45 @garethgreenaway