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

Pip: freeze all to be able to control setuptools, distribute, wheel, pip #418

Merged
merged 3 commits into from
Sep 7, 2018

Conversation

Feandil
Copy link
Contributor

@Feandil Feandil commented Jul 26, 2018

Pull Request (PR) description

By default pip freeze skip the mentioned packages and only displays their version if --all is used.
As a result, the following code would result in puppet trying to update the pip package every time:

    python::pip { 'update-pip' :
      ensure     => '10.0.1',
      pkgname    => 'pip',
      virtualenv => $virtualenv,
    }
Notice: /Stage[main]/.../Python::Pip[update-pip]/Exec[pip_install_update-pip]/returns: executed successfully

With this patch, this recurrent change disappears.

I've added --all to all cases, but only tested my own use case...

@bastelfreak
Copy link
Member

Hi @Feandil, thanks for the PR. I would like to wait with merging it until #419 is done.

@bastelfreak bastelfreak added the bug Something isn't working label Jul 29, 2018
@danquack
Copy link
Contributor

Is there any rspec tests that can be written to catch this from being regressed?

@bastelfreak
Copy link
Member

@Feandil can you please rebase and add an acceptance test for this change?

First part of regression test for PR#418
Second part of regression test for PR#418
By default `pip freeze` skip the mentioned packages and only displays
their version if `--all` is used
@Feandil
Copy link
Contributor Author

Feandil commented Sep 7, 2018

Sorry, I was away for a few weeks...

I've rebased on master and cut the regression test in two. Adding the fix, this makes it 3 commits:

  • c76c9b4 is the basic installation: it works with the existing code
  • bbb85bf check that puppet doesn't try to install it on every run: it fails with the existing code
  • 3c3ba03 is the fix I proposed

@bastelfreak
Copy link
Member

thanks for the patch!

@bastelfreak bastelfreak merged commit 2f16648 into voxpupuli:master Sep 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants