allow setting state for postgresql_databases and postgresql_users #360
Conversation
|
Interesting idea, but you need to fix the syntax errors, and I'd say remove the double quotes around the:
They aren't included elsewhere, so it goes against the style of this role. |
tests/vars.yml
Outdated
| @@ -18,6 +22,11 @@ postgresql_users: | |||
| pass: md51a1dc91c907325c69271ddf0c944bc72 | |||
| encrypted: yes | |||
|
|
|||
| - name: foo | |||
| pass: md51a1dc91c907325c69271ddf0c944bc72 | |||
| encrypted: yes | |||
There was a problem hiding this comment.
Do you need the password, etc to say that a user should be "absent"?
| @@ -17,6 +21,11 @@ postgresql_users: | |||
| pass: md51a1dc91c907325c69271ddf0c944bc72 | |||
| encrypted: yes | |||
|
|
|||
| - name: foo | |||
| pass: md51a1dc91c907325c69271ddf0c944bc72 | |||
There was a problem hiding this comment.
Do you need a password, etc. to say a user should be "absent"?
tasks/users.yml
Outdated
| @@ -5,13 +5,13 @@ | |||
| name: "{{ postgresql_service_name }}" | |||
| state: started | |||
|
|
|||
| - name: PostgreSQL | Make sure the PostgreSQL users are present | |||
| - name: "PostgreSQL | Make sure the PostgreSQL users are present" | |||
There was a problem hiding this comment.
Nowhere else in the role are double-quotes used, so please remove them to maintain the style
tasks/databases.yml
Outdated
| @@ -14,7 +14,7 @@ | |||
| lc_ctype: "{{ item.lc_ctype | default(postgresql_ctype) }}" | |||
| port: "{{postgresql_port}}" | |||
| template: "template0" | |||
| state: present | |||
| state: "{{ item.state | default(present)" | |||
There was a problem hiding this comment.
This is a syntax error, missing the closing }}. right? Can you please get it to run and pass the automated tests.
There was a problem hiding this comment.
thank you! I was looking at this thing for ages trying to figure out where my syntax error was. Figures it was right in front of my face.
README.md
Outdated
| @@ -83,6 +84,7 @@ postgresql_users: | |||
| - name: baz | |||
| pass: pass | |||
| encrypted: no # denotes if the password is already encrypted. | |||
| state: present # optional; one of 'present', 'absent' | |||
There was a problem hiding this comment.
If this is a string, then we should double-quote it. In general booleans are unquoted, but all strings should be quoted.
There was a problem hiding this comment.
Sounds good. Does it make sense to make the other string values double quoted in the README too?
There was a problem hiding this comment.
Yes, I believe so. I'd keep your changes to a minimum, and if you wanted to finish the cleanup of the variables then submit a separate PR. IMHO the only un-quoted strings should be when yes/no/true/false are used, so that they get cast to booleans.
README.md
Outdated
| @@ -68,6 +68,7 @@ postgresql_databases: | |||
| uuid_ossp: yes # flag to install the uuid-ossp extension on this database (yes/no) | |||
| citext: yes # flag to install the citext extension on this database (yes/no) | |||
| encoding: "UTF-8" # override global {{ postgresql_encoding }} variable per database | |||
| state: present # optional; one of 'present', 'absent', 'dump', 'restore' | |||
There was a problem hiding this comment.
If this is a string, then we should double-quote it. In general booleans are unquoted, but all strings should be quoted.
|
Thanks for the review @gclough! I've updated per your suggestions. |
|
You're welcome @mchitten. I'll ask that @jlozadad or @UnderGreen can review it for merge. I think it makes sense to be able to add as well as remove users, otherwise there is no cleanup possible. |
|
hey @gclough, any update on this? would love to use this for my projects .. thanks! |
allow setting state for postgresql_databases and postgresql_users
this allows configuring the state for databases and users. Default is present as it was previously.