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

Fix python::pip installing $editable VCS packages every Puppet run #491

Merged
merged 3 commits into from
Jun 10, 2019

Conversation

mlow
Copy link

@mlow mlow commented Jun 9, 2019

Pull Request (PR) description

This is due to VCS packages appearing in pip freeze -all as:
-e git://example.com/package.git@#egg=

Updates $grep_regex, which checks whether a given version is installed, to
match against the two different output formats of pip list. No longer
relies on pip freeze.

Additionally, this commit:

Removes the multiple exec resources which were mostly the same and replaces
with variables $pip_exec_name, $command, and $unless_command, which get
used in a single exec resource at the end of the manifest.

Checks if $pkgname contains ==, and if so splits based on == and sets
$real_pkgname to the first part, and $ensure variable to the second part.
Uses of $pkgname have been replaced with $real_pkgname.

This Pull Request (PR) fixes the following issues

Fixes #193

@mlow mlow changed the title Fix puppet::pip installing $editable VCS packages every Puppet run Fix python::pip installing $editable VCS packages every Puppet run Jun 9, 2019
@bastelfreak
Copy link
Member

Hi @wolttam, thanks for the PR. Can you add an acceptance test to verify that the bug is fixed? Please also have a look at the failed travis jobs.

@bastelfreak bastelfreak added bug Something isn't working needs-tests tests-fail labels Jun 9, 2019
@mlow mlow force-pushed the fix_pip branch 2 times, most recently from f579fa2 to 33bb9f7 Compare June 9, 2019 19:35
@mlow
Copy link
Author

mlow commented Jun 9, 2019

I fixed what was causing the existing tests to fail -- missing quotes (oops).

And, I added a new test for this, except, here's a quandary... git (or some other vcs, but git seems to be the appropriate choice) is required for it to run. I'm quite unfamiliar with the test framework in use here, so I'm not sure how to make git available. I'm looking into it, though.

@bastelfreak
Copy link
Member

bastelfreak commented Jun 9, 2019

this will install git within the docker environment (and improves the formatting):

diff --git a/spec/acceptance/virtualenv_spec.rb b/spec/acceptance/virtualenv_spec.rb
index a3520c3..dbcfd6d 100644
--- a/spec/acceptance/virtualenv_spec.rb
+++ b/spec/acceptance/virtualenv_spec.rb
@@ -113,21 +113,22 @@ describe 'python class' do
     end
     it 'works with editable=>true' do
       pp = <<-EOS
-      class { 'python' :
+      package{'git':
+        ensure => 'present',
+      }
+      -> class { 'python' :
         version    => 'system',
         pip        => 'present',
         virtualenv => 'present',
       }
-      ->
-      python::virtualenv { 'venv' :
+      -> python::virtualenv { 'venv' :
         ensure     => 'present',
         systempkgs => false,
         venv_dir   => '/opt/venv5',
         owner      => 'root',
         group      => 'root',
       }
-      ->
-      python::pip { 'rpyc' :
+      -> python::pip { 'rpyc' :
         ensure     => '4.1.0',
         url        => 'git+https://github.com/tomerfiliba/rpyc.git',
         editable   => true,

@mlow
Copy link
Author

mlow commented Jun 9, 2019

this will install git within the docker environment (and improves the formatting):
....

Of course! Added that, pushing now.

@mlow mlow force-pushed the fix_pip branch 2 times, most recently from 174d3c4 to e124fb5 Compare June 9, 2019 20:47
@mlow mlow force-pushed the fix_pip branch 2 times, most recently from b29877e to cbd8356 Compare June 10, 2019 08:19
mlow added 3 commits June 10, 2019 12:35
This is due to VCS packages appearing in `pip freeze --all` as:
-e git://example.com/package.git@<ref>#egg=<egg>

Updates $grep_regex, which checks whether a given version is installed, to
match against the two different output formats of `pip list`. No longer
relies on pip freeze.

Additionally, this commit:

- Removes the multiple exec resources which were mostly the same and replaces
with variables $pip_exec_name, $command, and $unless_command, which get
used in a single exec resource at the end of the manifest.

- Checks if $pkgname contains ==, and if so splits based on == and sets
$real_pkgname to the first part, and $_ensure to the second part. Uses of
$pkgname have been replaced with $real_pkgname.

Fixes voxpupuli#193
Improve formatting of existing tests
@bastelfreak
Copy link
Member

Thanks for the awesome work!

@bastelfreak bastelfreak merged commit 4aa2f62 into voxpupuli:master Jun 10, 2019
@mlow mlow deleted the fix_pip branch June 10, 2019 10:49
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.

Installing from git repo runs install on every Puppet run
2 participants