diff mbox series

[bug#35544] services: dovecot: Rename auth-verbose-passwords?.

Message ID 20190503091039.12424-1-mail@cbaines.net
State Accepted
Headers show
Series [bug#35544] services: dovecot: Rename auth-verbose-passwords?. | expand

Checks

Context Check Description
cbaines/applying patch success Successfully applied

Commit Message

Christopher Baines May 3, 2019, 9:10 a.m. UTC
* gnu/services/mail.scm (dovecot-configuration)[auth-verbose-passwords?]:
Rename to auth-verbose-passwords, and change the type to a string, as this
parameter can take one of three string values.
* doc/guix.texi (Dovecot service): Update the corresponding documentation.
---
 doc/guix.texi         | 4 ++--
 gnu/services/mail.scm | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

Comments

Ludovic Courtès May 7, 2019, 1:58 p.m. UTC | #1
Hello!

Christopher Baines <mail@cbaines.net> skribis:

> * gnu/services/mail.scm (dovecot-configuration)[auth-verbose-passwords?]:
> Rename to auth-verbose-passwords, and change the type to a string, as this
> parameter can take one of three string values.
> * doc/guix.texi (Dovecot service): Update the corresponding documentation.

I don’t use the Dovecot service but this LGTM.

The question is whether it’s OK to break the API.  I’d say that with
proper documentation it probably is.  Thoughts?

Longer-term we’ll need a way to gracefully handle deprecation for this
kind of change, probably at the level of the ‘define-record-type*’
kitchen sink.

Thanks,
Ludo’.
Christopher Baines May 8, 2019, 7:21 a.m. UTC | #2
Ludovic Courtès <ludo@gnu.org> writes:

> Christopher Baines <mail@cbaines.net> skribis:
>
>> * gnu/services/mail.scm (dovecot-configuration)[auth-verbose-passwords?]:
>> Rename to auth-verbose-passwords, and change the type to a string, as this
>> parameter can take one of three string values.
>> * doc/guix.texi (Dovecot service): Update the corresponding documentation.
>
> I don’t use the Dovecot service but this LGTM.

Great, I've pushed this now.

> The question is whether it’s OK to break the API.  I’d say that with
> proper documentation it probably is.  Thoughts?
>
> Longer-term we’ll need a way to gracefully handle deprecation for this
> kind of change, probably at the level of the ‘define-record-type*’
> kitchen sink.

Yeah, I'm uncertain. For long running systems, it's probably good to
update the packages, without having to adjust the service configuration
for changes like this. If there was a "stable" channel to track, which
didn't include updates to services, but did include important package
updates, then that may be useful.

Also, just making the errors relating to service configuration may be
more impactful than adding extra deprecation support.

Thanks,

Chris
Ludovic Courtès May 8, 2019, 10:43 a.m. UTC | #3
Christopher Baines <mail@cbaines.net> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:

[...]

>> The question is whether it’s OK to break the API.  I’d say that with
>> proper documentation it probably is.  Thoughts?
>>
>> Longer-term we’ll need a way to gracefully handle deprecation for this
>> kind of change, probably at the level of the ‘define-record-type*’
>> kitchen sink.
>
> Yeah, I'm uncertain. For long running systems, it's probably good to
> update the packages, without having to adjust the service configuration
> for changes like this. If there was a "stable" channel to track, which
> didn't include updates to services, but did include important package
> updates, then that may be useful.
>
> Also, just making the errors relating to service configuration may be
> more impactful than adding extra deprecation support.

The problem, as I see it, is that possibly weeks from now people will
try to reconfigure and will get an error about
‘auth-verbose-passwords?’.  At that point they’ll have to dig to figure
out that there’s a field with a similar name and similar semantics and
to adjust their code accordingly.

But maybe the real solution is providing a “news” system, as discussed
with Tobias and others recently on guix-devel: ‘guix pull -N’ would
display a message saying that the Dovecot API has changed, etc.

Ludo’.
diff mbox series

Patch

diff --git a/doc/guix.texi b/doc/guix.texi
index 7cda06de5c..1fe4618742 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -15845,13 +15845,13 @@  failed.
 Defaults to @samp{#f}.
 @end deftypevr
 
-@deftypevr {@code{dovecot-configuration} parameter} boolean auth-verbose-passwords?
+@deftypevr {@code{dovecot-configuration} parameter} string auth-verbose-passwords
 In case of password mismatches, log the attempted password.  Valid
 values are no, plain and sha1.  sha1 can be useful for detecting brute
 force password attempts vs.  user simply trying the same password over
 and over again.  You can also truncate the value to n chars by appending
 ":n" (e.g.@: sha1:6).
-Defaults to @samp{#f}.
+Defaults to @samp{"no"}.
 @end deftypevr
 
 @deftypevr {@code{dovecot-configuration} parameter} boolean auth-debug?
diff --git a/gnu/services/mail.scm b/gnu/services/mail.scm
index 0dabfed4cb..216b2c80b0 100644
--- a/gnu/services/mail.scm
+++ b/gnu/services/mail.scm
@@ -806,8 +806,8 @@  standard facilities are supported.")
    "Log unsuccessful authentication attempts and the reasons why they
 failed.")
 
-  (auth-verbose-passwords?
-   (boolean #f)
+  (auth-verbose-passwords
+   (string "no")
    "In case of password mismatches, log the attempted password.  Valid
 values are no, plain and sha1.  sha1 can be useful for detecting brute
 force password attempts vs.  user simply trying the same password over