search

saltstack-formulas / postgres-formula

http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
77 stars 283 forks source link

Use copy state instead of salt-call #256

Closed thomasrossetto closed 3 years ago

thomasrossetto commented 5 years ago

Now it works also with salt-ssh.

thomasrossetto commented 5 years ago

Hi @vutny ! I tried to replace with prereq instead of requireand with salt-ssh i don't have any problems. But, i don't able to test if this is the correct replace (according to your CR) for salt-call command.

vutny commented 5 years ago

No worries @thomasrossetto !

Just update the PR with your new changes and I think we'd make it through. Thank you for your efforts.

vutny commented 5 years ago

@thomasrossetto Would you be able to finish this PR and address the comments? Your changes are great, we just need to resolve the case where local config backup is explicitly disabled. This is long time legacy, and by removing it you will significantly improve the formula. Thanks for your work and contributions here!

myii commented 5 years ago

Good work @thomasrossetto! I've been working on getting the Travis-based Kitchen tests established for this formula but the fedora-28 instance was failing.

https://travis-ci.org/myii/postgres-formula/jobs/524280903#L1427:

                 ID: postgresql-pg_hba
           Function: file.managed
               Name: /var/lib/pgsql/data/pg_hba.conf
             Result: False
            Comment: check_cmd execution failed

Applied these commits and it's now working.

https://travis-ci.org/myii/postgres-formula/jobs/524291445#L1439:

                 ID: postgresql-pg_hba
           Function: file.managed
               Name: /var/lib/pgsql/data/pg_hba.conf
             Result: True
            Comment: File /var/lib/pgsql/data/pg_hba.conf updated

While this might not be what your intention was when you submitted this PR, it's a nice bonus.

myii commented 5 years ago

@vutny After setting up the CI PR (#262), the fedora-28 test is now passing, even without this merge.

https://travis-ci.com/saltstack-formulas/postgres-formula/jobs/195597494#L1552:

                 ID: postgresql-pg_hba
           Function: file.managed
               Name: /var/lib/pgsql/data/pg_hba.conf
             Result: True
            Comment: File /var/lib/pgsql/data/pg_hba.conf updated

So no need to merge this PR prematurely, it seems.

vutny commented 5 years ago

Great, thank you @myii !

noelmcloughlin commented 3 years ago

Thanks, @thomasrossetto @myii @vutny