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

python::requirements interferes with managing requirements.txt if not explicitly in a file resource #64

Closed
yungchin opened this issue Mar 5, 2014 · 3 comments

Comments

@yungchin
Copy link

yungchin commented Mar 5, 2014

Hello, thanks for this super-useful module. I'd like to make a change to this block here
https://github.com/stankevich/puppet-python/blob/0277b9c81f5838dce9353a0a87a7029b7ebcf397/manifests/requirements.pp#L92
...but have some questions about how to best solve it.

Here's the situation: we deploy a package ("coolstuff") that has its own requirements.txt, like so:

file { "/srv/coolstuff":
    ensure  => directory,
    recurse => true,
    source  => "puppet:///modules/coolstuff/coolstuff",
}

python::virtualenv { "/srv/coolstuff_venv":
    requirements => "/srv/coolstuff/requirements.txt",
    require      => File["/srv/coolstuff"],
}

Result: the block in requirements.pp, which as linked above has "replace => false", locks the requirements.txt down at whatever version first appeared. So if we update the file in the coolstuff module, that newer version never gets deployed.

The workaround is to make it explicit that we manage the file ourselves, ie we add:

file { "/srv/coolstuff/requirements.txt":
    source  => "puppet:///modules/coolstuff/coolstuff/requirements.txt",
}

but obviously that's a bit ugly.

Would there be any other way to detect that requirements.txt is managed elsewhere, or should I rather add an attribute "manage_requirements (true/false)" to python::requirements (and also to python::virtualenv) to prevent this manually?

(Frankly, I don't really see the use case where python::requirements should ever manage requirements.txt, so as far as I'm concerned, it should be fine if that whole block, L83-94, goes out altogether?)

@stankevich
Copy link
Collaborator

The workaround is to make it explicit that we manage the file ourselves

I think this would be the way to do it in this case. It's not pretty but other solutions could be even worse. Unless you have something in the works already?

@yungchin
Copy link
Author

yungchin commented Mar 7, 2014

Hi, thanks for looking at this. So, uhm... would you be open to a patch removing the whole of this if-block? The thing is, I don't really understand how I would want to use python::requirements and not provide my own requirements.txt in some way or other, so I don't understand what that block helps for?

(edit: I do suppose it would be a bit nasty, because this change might break some people's manifests if they didn't have a require/subscribe pointing to wherever the requirements.txt file was provided from)

(edit: but also, note that my workaround above is harder to pull off if the requirements.txt came from a vcs repo of some sort)

@wimh
Copy link
Contributor

wimh commented Jul 18, 2014

I am new to puppet, and I was quite surprised that something like this does not work as expected (by me);

file { "/srv/coolstuff":
    ensure  => directory,
    recurse => true,
    source  => "puppet:///modules/coolstuff/coolstuff",
}

python::virtualenv { "/srv/coolstuff_venv":
    requirements => "/srv/coolstuff/requirements.txt",
    require      => File["/srv/coolstuff"],
}

It would create a new requirements.txt file instead of copy the file. And if I would use an existing file which does not need to be copied, it would change owner and group to root.

But now I understand the difficulties better than before. If the requirements.txt is a managed file resource, it will also be possible to detect changes and rerun pip install if it has changed. So it would be a good habit to manage that file, even if it is currently not needed.

But it might be possible to improve the current content of the generated file;

content => '# Puppet will install and/or update pip packages listed here',

for example (add)

content => '# Puppet has generated this file as you did not specify your own file resource in your project',

For newbies like I was, it would make it easier to understand what is happening.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants