[PATCH v4] Add some properties to install and configure Postgresql

[Sean Whitton]

Hello,

Thanks for this.

On Sun 23 Jan 2022 at 01:25PM -04, David Bremner wrote:

> The repetition of 'as' is because it applies to entire properties, not
> e.g. :check forms.

You could avoid it if you would like to by wrapping %RUN-SQL in a
DEFPROPLIST, and that would also eliminate repetition of INSTALLED.  But
perhaps overall less readable.  Up to you.

> diff --git a/src/property/postgres.lisp b/src/property/postgres.lisp
> new file mode 100644
> index 0000000..2d1cc11
> --- /dev/null
> +++ b/src/property/postgres.lisp
> @@ -0,0 +1,98 @@
> +;;; Consfigurator -- Lisp declarative configuration management system
> +
> +;;; Copyright (C) 2021  David Bremner <david at tethera.net>

I think 2021--2022 now?

> +(defprop %run-sql :posix (sql &key unless)
> +  (:check
> +   (declare (ignore sql))
> +   (or
> +    (null unless)
> +    (let ((result (string-trim '(#\Space #\Newline #\Tab #\Return)
> +                               (mrun "psql" "-t" "postgres" :input unless))))

Looks like if there is no :UNLESS then this :CHECK subroutine will
always return true, and so the :APPLY routine will never be called.  I
think you want something like

    (and unless (alet (string-trim ...) ...))

(I like ALET, no problem if you don't!)

> +      (informat 4 "~&PSQL=> ~a~%" result)

Do you really need both the ~& and the ~%?  Typically calls to INFORMAT
use ~& at the beginning but not ~% at the end.

> +      (string-equal "yes" result))))

Also, it's very minor, but as you don't need the case-insensitive
matching I'd prefer using STRING= to STRING-EQUAL.

> +(defproplist has-owner :posix (database owner)
> +  (:desc #?"Postgres database ${database} has owner ${owner}")
> +  (installed)
> +  (as (super-user)
> +    (%run-sql #?"ALTER DATABASE ${database} OWNER TO ${owner}"
> +              :unless (strcat "select 'yes' from pg_database d, pg_authid a"
> +                              #?" where d.datname='${database}' and d.datdba = a.oid"
> +                              #?" and a.rolname='${owner}';"))))

I find it a bit confusing to have both #? and STRCAT in use at once.
How about just using the former?  You could still line things up with
whitespace, I would assume.

-- 
Sean Whitton