-
Notifications
You must be signed in to change notification settings - Fork 282
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(repo): reorder dependencies to prevent errors #309
Conversation
This is the exact contrary of this #300
Not sure of what I am saying, I had the exact same problem today but this PR need to be double checked |
postgres/server/init.sls
Outdated
@@ -20,6 +20,8 @@ postgresql-server: | |||
- pkgs: {{ pkgs | json }} | |||
{%- if postgres.use_upstream_repo == 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.
This statement probably needs additional expression, the same as in upstream.sls
:
{%- if postgres.use_upstream_repo == true %} | |
{%- if 'pkg_repo' in postgres and postgres.use_upstream_repo == true %} |
Because if "upstream" is flag is on, but there are no repo definition for particular distro, the requisite will fail.
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.
@sticky-note, I added the changes @vutny suggests and also added the same check in a few other places where postgres.use_upstream_repo
was being checked but pkg_repo
was not.
I think the changes in this PR should be OK for all the platforms, but I don't have a FreeBSD around to try it. Can you try it and give us your feedback?
70d224e
to
d5b11c9
Compare
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.
Thanks @javierbertoli , I just have minor comment about MacOS case.
d5b11c9
to
0d45422
Compare
0d45422
to
26b2233
Compare
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.
Looks good to me, thank you @javierbertoli !
Ok, it seems to have no issue in this manner. Thanks @javierbertoli |
Thanks to you both! |
🎉 This PR is included in version 0.41.6 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
PR progress checklist (to be filled in by reviewers)
What type of PR is this?
Primary type
[build]
Changes related to the build system[chore]
Changes to the build process or auxiliary tools and libraries such as documentation generation[ci]
Changes to the continuous integration configuration[feat]
A new feature[fix]
A bug fix[perf]
A code change that improves performance[refactor]
A code change that neither fixes a bug nor adds a feature[revert]
A change used to revert a previous commit[style]
Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc.)Secondary type
[docs]
Documentation changes[test]
Adding missing or correcting existing testsDoes this PR introduce a
BREAKING CHANGE
?No.
Related issues and/or pull requests
Describe the changes you're proposing
When installing a
postgresql-client
using upstream's repo, the formula fails to install the repo because arequire_in
dependency expects thepostgresql-server
package. This PR moves the dependency to theserver/init.sls
state.Pillar / config required to test the proposed changes
The issue can be reproduced using this pillar
and this command
Debug log showing how the proposed changes work
Documentation checklist
README
(e.g.Available states
).pillar.example
.Testing checklist
state_top
).Additional context