-
Notifications
You must be signed in to change notification settings - Fork 419
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
ENH: Add install_from_ppa pillar setting and PPA pkgrepo #42
ENH: Add install_from_ppa pillar setting and PPA pkgrepo #42
Conversation
While nginx. classic does not seem to be working completely correctly (w/ Ubuntu 12.04), I did add the PPA to both:
|
ENH: Add install_from_ppa pillar setting and PPA pkgrepo
Thanks!
|
@westurner You're welcome. Thanks for the good work! |
👍 |
@@ -35,9 +36,23 @@ nginx-old-init-disable: | |||
- file: nginx-old-init | |||
{% endif %} | |||
|
|||
{% if salt['grains.get']('os_family') == 'Debian' %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this always add the PPA even if install_from_ppa
is False
? Is that desirable? It probably doesn't hurt anything, but would it be better to only add the PPA if planning to install from it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@westurner Can you create a pull request with a check that @gtback suggests? I saw you did a check in nginx/ng/install.sls
but probably forgot to do it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without enabled = False
, the pkgrepo would remain enabled in the event that the boolean changes from True to False.
Should I add the 'enabled' attr from Line 42 (package) to Line ~20 (ng.install)?:
enabled: {{ salt['pillar.get']('nginx:install_from_ppa', False) }}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On further review, this is more complex to implement than I considered. I apologize.
Does this look correct (nginx.install_from_ppa: True/False)?:
- False: pkg.install
- True: pkgrepo.managed, pkg.install
- False -> True: pkgrepo.managed, pkg.reinstall
- True -> False: pkgrepo.absent, pkg.reinstall
Is it reasonable to expect an implementer to call pkg.reinstall (install, reinstall=True)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Posted to salt-users: https://groups.google.com/forum/#!topic/salt-users/ZdglOhvecSo
@westurner I reverted the changes made from this pull request. Figured it's not the correct way to go about it. |
No description provided.