If user declares their requirements.txt in Puppet, don't skip pip installation in python::requirements

[acullenn]

trafficstars

@smortex

Pull Request (PR) description

Fixes issue wherein the python::requirements class does not correctly install a requirements.txt file declared in Puppet to the declared venv, e.g.

     file { "${project_dir}/requirements.txt":
        ensure  => present,
        content => file("${module_name}/requirements.txt")
      }
    -> python::pyvenv { "${project_dir}/pyenv":
      ensure  => present,
      version => '3.6'
      }
    -> python::requirements {"${project_dir}/requirements.txt":
        virtualenv => "${project_dir}/pyenv",
      }

This appears to be due to the logic for setting the local resource subscription - it checks if the requirements file is not declared before setting the local subscription which ultimately gates the pip installation. This behavior runs contrary to the provided Example(s), in which requirements.txt must exist or be declared in advance of invoking the class: https://forge.puppet.com/modules/puppet/python/reference#pythonrequirements

python::requirements { '/var/www/project1/requirements.txt' :
  virtualenv => '/var/www/project1',
  proxy      => 'http://proxy.domain.com:3128',
  owner      => 'appuser',
  group      => 'apps',
}

Changes

• Always set the local subscription in python::requirements, which is used to trigger the pip installation.

This Pull Request (PR) fixes the following issues

Fixes #613

[smortex]

Ah. I feel like I encountered this at some point, but I was not able to reproduce my failing test after refactoring so assumed I added a missing dependency somewhere…

I would have expected these changes to break tests where multiple venv share the same requirements.txt file but maybe we do not test this case in CI.

Yet, I do not feel this is the right fix since we want to trigger a new install if the requirements file is changed and not managed by Puppet (e.g. it is part of a repo managed by vcsrepo). I think we now fail here (and it seems not to not have a corresponding test).

Can you add a (failing) unit test that demonstrate the problem you want to fix so that I have a way to reproduce this and think about it?

[acullenn]

I've added a unit test for this issue, and confirmed that it's failing when my fix is not applied:(https://github.com/acullenhms/puppet-python/pull/2)

But it succeeds when the subscription changes are in place. Interestingly though, The Puppet + Ruby 2.6/2.7 test fail when the fix exists, but succeed when it isn't. I'd be interested to know why.

This is my first Ruby +/ Puppet unit test, so if I've made a mistake or style flub please let me know!

I would have expected these changes to break tests where multiple venv share the same requirements.txt file but maybe we do not test this case in CI.

My understanding of the codebase is incomplete - what about the logic for requirements installation would make you expect this?

[vox-pupuli-tasks[bot]]

Dear @acullenhms, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

[saz]

@smortex showed a valid reason, why local_subscribe might be set to undefined. I'm using vcsrepo which clones a repo with the requirements.txt included, which means, that there's no File[$requirements] defined in all cases.

But: this setup won't trigger any installation, as long as forceupdate isn't set to true, as refreshonly won't help here (there's nothing refreshing anything - I can set this up manually on the vcsrepo part).

There's a --dry-run option added in pip 22.2 (which is the newest release), which might help here - in the future. A way to check, if there are uninstalled packages would be the easiest way to resolve this issue, but as far as I know, there's no such thing.

python3 -c "import pkg_resources; pkg_resources.require(open('requirements.txt',mode='r'))" &>/dev/null

seems to work, but it fails, if there's a mismatch within the package name. E.g.

zope-interface==5.4.0  # requirements.txt
zope.interface==5.4.0  # pip freeze

Looks like, as if pip does some magic on the package names (- vs _, - vs .,...)

[siebrand]

Is there any way to get this patch going again?

[zheng460]

Hello, I used Puppet to manage my requirement.txt. This module won't subscribe to the requirement.txt. I have a similar fix that I would like to apply. Is there anything I can do to help get this going?