GNU bug report logs - #33549
[PATCH 0/6] Add Zabbix services.

Previous Next

Full log

Message #34 received at 33549 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Oleg Pykhalov <go.wigust <at> gmail.com>
Cc: 33549 <at> debbugs.gnu.org
Subject: Re: [bug#33549] [PATCH 6/6] services: monitoring: Add
 'zabbix-front-end'.
Date: Wed, 19 Dec 2018 16:27:37 +0100

Hi Oleg,

Oleg Pykhalov <go.wigust <at> gmail.com> skribis:

> +(define zabbix-front-end-config
> +  (match-lambda
> +    (($ <zabbix-front-end-configuration>
> +        _ db-host db-port db-name db-user db-password db-secret-file
> +        zabbix-host zabbix-port)
> +     (mixed-text-file "zabbix.conf.php"
> +                      "\
> +<?php
> +// Zabbix GUI configuration file.
> +global $DB;
> +
> +$DB['TYPE']     = 'POSTGRESQL';
> +$DB['SERVER']   = '" db-host "';
> +$DB['PORT']     = '" (number->string db-port) "';
> +$DB['DATABASE'] = '" db-name "';
> +$DB['USER']     = '" db-user "';
> +$DB['PASSWORD'] = '" (if (string-null? db-password)
> +                         (if (string-null? db-secret-file)
> +                             (display "Provide a `db-secret-file' \
> +or `db-password' field.
> +"
> +                                      (current-error-port))
> +                             (string-trim-both
> +                              (with-input-from-file db-secret-file
> +                                read-string)))
> +                         (begin
> +                           (display "
> +Hint: Consider use `db-secret-file' instead of `db-password' and unset
> +`db-password' in `zabbix-front-end-configuration' for security.
> +")
> +                           db-password)) "';
> +
> +// Schema name. Used for IBM DB2 and PostgreSQL.
> +$DB['SCHEMA'] = '';
> +
> +$ZBX_SERVER      = '" zabbix-host "';
> +$ZBX_SERVER_PORT = '" (number->string zabbix-port) "';
> +$ZBX_SERVER_NAME = '';
> +
> +$IMAGE_FORMAT_DEFAULT = IMAGE_FORMAT_PNG;
> +"))))

I saw these “hints” in the build log of Cuirass and that got me curious.
:-)

A couple of comments…  For consistency, hints should be reported using
the ‘display-hint’ procedure, which takes a Texinfo string as input.

Second, while the second ‘display’ call does look like a hint, the first
one looks like it should be an error, shouldn’t it?  In that case, I’d
suggest something like:

  (raise (condition
           (&message
            (message "You must provide either 'db-secret-file' or 'db-password'."))))

Third, it would be nice to report source location info along with hints
and errors.  To do that, you could add an innate ‘location’ field to
<zabbix-front-end-configuration> as done for <package>.  Then, along
with the &message condition above, you could raise an &error-location as
is done in a few places.

Does that make sense?

Anyway thanks for this patch series, we’ll probably put it to good use
on the build farm!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.