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

Add role for generically installing an apt package #83

Merged
merged 2 commits into from
Nov 18, 2016

Conversation

philwills
Copy link
Contributor

This was driven by wanting pip pre-installed to avoid dpkg lock contention with cloud watch logs, but it seems like this would be generally useful.

Not at all sure about the name, so would love alternative suggestions.

@davidfurey
Copy link
Member

Great idea. Can you use this to install multiple packages?

@philwills
Copy link
Contributor Author

@davidfurey Good question, I'll have a look and see. Would seem silly not to be able to.

@sihil
Copy link
Contributor

sihil commented Nov 15, 2016

I'm not entirely against this, but it seems like it could be subject to prolific undocumented use. this doesn't enable any new abilities (we can achieve this with a role that, implicitly or explictly, documents intent).

Is this really saying that roles are too difficult to add?

@kenoir
Copy link
Contributor

kenoir commented Nov 15, 2016

+1 what @sihil said. I think it would be worth putting work into extracting the roles from the code and making them easier to modify (i.e. not requiring a deploy).

I've created an issue for this: #86

@philwills
Copy link
Contributor Author

@sihil I think they're enough of a pain that it's reasonable to avoid for the simplest cases, e.g. install this one package. Even for someone like me who's used to the process, that would be an hour or two of effort plus considerably more elapsed time, compared to 5 minutes with this available.

I think apt-install with a a parameter of python-pip is no less self-documenting than a role called, pip (deb-based distros)

@sihil
Copy link
Contributor

sihil commented Nov 15, 2016

Except that's doesn't seem like a good role name IMHO. A role is about the task you want to use it for. I guess the recipe name is useful here and each recipe could potentially have a notes or description field too.

@philwills
Copy link
Contributor Author

Also, I'm really appreciative of all the feedback and discussion of this.

@philwills
Copy link
Contributor Author

@sihil I'm not sure what you mean. I don't understand how the existing roles fit that description.

@sihil
Copy link
Contributor

sihil commented Nov 15, 2016

Yeah - fair enough - I've just reviewed them and there isn't really anything special there. I guess my concern is that people eventually stop doing roles entirely by creating a class of roles that let them install packages, files, etc etc with ever more complex recipes. Maybe that's OK, but I'd rather make roles easier to edit.

I propose that we do this and see what happens.

@philwills philwills force-pushed the apt-install-role branch 4 times, most recently from 0511ecc to 86e8b39 Compare November 17, 2016 12:33
Also made the task work with yum on RedHat derivatives
@philwills
Copy link
Contributor Author

I've updated the task with a more declarative name, which suggested itself when adding support for RedHat derivatives. I've also added support for installing multiple packages which has meant introducing a small parser to replace a regular expression.

@philmcmahon
Copy link
Contributor

Brilliant! Code looks good to me 👍

@philwills philwills merged commit 6fb4fd3 into master Nov 18, 2016
@philwills philwills deleted the apt-install-role branch November 18, 2016 14:38
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

Successfully merging this pull request may close these issues.

5 participants