Re: [PATCH] gnu: Add postgresql-service.

[Ludovic Courtès]

David Thompson <address@hidden> skribis:

> From 888bb58253ab708f38609aa839e77a46a96bd0df Mon Sep 17 00:00:00 2001
> From: David Thompson <address@hidden>
> Date: Sun, 3 May 2015 17:02:59 -0400
> Subject: [PATCH] gnu: Add postgresql-service.
>
> * gnu/services/databases.scm: New file.
> * gnu-system.am (GNU_SYSTEM_MODULES): Add it.
> * doc/guix.texi ("Database Services"): New subsubsection.

Nice!  I haven’t tested it, but it looks good.  I only have minor
comments:

> +;;; Copyright © 2015

Line truncated.  :-)

> +(define %default-postgres-hba
> +  (text-file* "pg_hba.conf"
> +              "local all     all                     trust\n"
> +              "host  all     all     127.0.0.1/32    trust\n"
> +              "host  all     all     ::1/128         trust\n"))
> +
> +(define %default-postgres-ident
> +  (text-file* "pg_ident.conf"
> +              "# MAPNAME       SYSTEM-USERNAME         PG-USERNAME"))

Since these two are purely text (they do not contain references to store
items), ‘text-file’ is enough and even better.

(‘text-file*’ returns a derivation that builds the file, and that
derivation depends on Guile; conversely, ‘text-file’ simply creates a
new file in the store.)

> +          ;; Drop privileges and init state directory in a new
> +          ;; process.  Wait for it to finish before proceeding.
> +          (match (primitive-fork)
> +            (0
> +             (setgid (passwd:gid user))
> +             (setuid (passwd:uid user))
> +             (primitive-exit (system* initdb "-D" #$data-directory)))

It may be a good idea to wrap the child process’s body in:

  (dynamic-wind
    (const #t)
    (lambda ()
      ...)
    (lambda ()
      (primitive-exit 1)))

This makes sure that it doesn’t keep going if an exception is thrown
(yes it’s quite ugly.)

OK to push with these changes!

Thanks,
Ludo’.