[PATCH 6 of 6] e-mail: add documentation about configuration settings

Thu Aug 6 12:16:02 UTC 2015

On Mon, Aug 3, 2015 at 12:28 AM, Mads Kiilerich <mads at kiilerich.com> wrote:
> On 08/02/2015 10:51 PM, Thomas De Schampheleire wrote:
>>
>> # HG changeset patch
>> # User Thomas De Schampheleire <thomas.de.schampheleire at gmail.com>
>> # Date 1437061235 -7200
>> #      Thu Jul 16 17:40:35 2015 +0200
>> # Node ID b6ead0eb65812005d1b5a5893f54efb1f08ee079
>> # Parent  b2edf3d7ad971e48214632bba5187c490161716d
>> e-mail: add documentation about configuration settings
>
>
> Very nice!
>
>
>> Document e-mail related configuration settings in the documentation
>> (overview) and the .ini file itself (technical details and example
>> settings).
>>
>> This includes a replacement of @localhost examples to @example.com, to
>> indicate that real addresses should normally be used.
>>
>> diff --git a/development.ini b/development.ini
>> --- a/development.ini
>> +++ b/development.ini
>> @@ -16,16 +16,37 @@ debug = true
>>   pdebug = false
>>
>> ################################################################################
>> -## Uncomment and replace with the address which should receive
>> ##
>> -## any error reports after application crash
>> ##
>> -## Additionally those settings will be used by Kallithea mailing system
>> ##
>> +## E-mail settings
>> ##
>> +##
>> ##
>> +## Refer to the documentation ("E-mail settings") for more details.
>> ##
>> +##
>> ##
>> +## It is recommended to use a valid sender address that passes access
>> ##
>> +## validation and spam filtering in mail servers.
>> ##
>>
>> ################################################################################
>> -#email_to = admin at localhost
>> -#error_email_from = paste_error at localhost
>> -#error_subject_prefix =
>> -#app_email_from = kallithea-noreply at localhost
>> +
>> +## From header for application e-mails. You can optionally add a name.
>> Examples:
>
>
> It would be easier to read this line of 'From' was quoted somehow.
>
>> +#app_email_from = Kallithea <kallithea-noreply at example.com>
>> +#app_email_from = kallithea-noreply at example.com
>> +
>> +## Subject prefix for application e-mails.
>> +## A space between this prefix and the real subject is automatically
>> added.
>> +## Example:
>>   #email_prefix = [Kallithea]
>>   +## Recipients for error e-mails and fallback recipients of application
>> mails.
>> +## Multiple addresses can be specified, space-separated.
>> +## Only addresses are allowed, do not add any name part.
>> +#email_to = admin at example.com
>> +
>> +## From header for error e-mails. You can optionally add a name.
>> Examples:
>> +#error_email_from = Kallithea Errors <kallithea-noreply at example.com>
>> +#error_email_from = paste_error at example.com
>> +
>> +## Subject prefix for error e-mails.
>> +## Standard trailing spaces are trimmed, use a Unicode variant like
>> U+2002
>> +## instead. For example (there is a trailing U+2002 space after ]):
>> +#error_subject_prefix = [Kallithea error]
>> +
>
>
> Would it (at least for some of the options such as the prefixes) be a good
> idea to document what the default value is?

I added these now, feel free to format it in a different way...

>
>
>>   #smtp_server = mail.server.com
>>   #smtp_username =
>>   #smtp_password =
>> diff --git a/docs/index.rst b/docs/index.rst
>> --- a/docs/index.rst
>> +++ b/docs/index.rst
>> @@ -37,6 +37,7 @@ Kallithea Documentation
>>   .. toctree::
>>      :maxdepth: 1
>>   +   usage/e-mail
>>      usage/performance
>>      usage/backup
>>      usage/debugging
>> diff --git a/docs/usage/e-mail.rst b/docs/usage/e-mail.rst
>> new file mode 100644
>> --- /dev/null
>> +++ b/docs/usage/e-mail.rst
>> @@ -0,0 +1,61 @@
>> +.. _email:
>> +
>> +===============
>> +E-mail settings
>> +===============
>> +
>> +The Kallithea configuration file has several e-mail related settings.
>> When these
>> +contain correct values, Kallithea will send e-mail in the situations
>> described
>> +below.
>> +
>> +Application e-mails
>> +-------------------
>> +
>> +Kallithea sends an e-mail to `users` on several occasions:
>> +
>> +- when comments are given on one of their changesets
>> +- when comments are given on changesets they also commented on
>
>
> or are reviewers on
>
>> +- when they are invited as reviewer in pull requests
>> +- when they request a password reset
>> +
>> +Kallithea sends an e-mail to all `administrators` upon new account
>> registration.
>> +Administrators are users with the ``Admin`` flag set in the
>> ``Admin->Users``
>> +section.
>> +
>> +In case Kallithea tries to send an e-mail to unspecified recipients, the
>
>
> "unspecified recipients" sounds a bit confusing at this place in the text.
> Perhaps give a hint like "when an error situation occurs and Kallithea
> doesn't know where to send the mail, ..." ... or be more specific.
>
>> +administrators and the addresses specified in ``email_to`` in the
>> configuration
>> +file are used as fallback.
>> +
>> +The ``From`` header of these e-mails is set using the ``app_email_from``
>> setting
>> +in the configuration file. The ``Subject`` of these e-mails can
>> optionally be
>> +prefixed with the value of ``email_prefix`` in the configuration file.
>
>
> I appreciate the conceptual clarity of what kind of sender address it is,
> but in a user facing documentation like this, I think it would be better to
> explain it and talk about "the recipient will see the mail as coming from".
> As far as I know, it is also the case that "app_email_from is expected to be
> in 'User Name <username at example.com>' format and will thus both control the
> name most email programs will show and also the actual address that will be
> used if the user should try to reply" - if that really is the case, I think
> we should say something like that.
>
> (Hmm ... your fine documentation in the .ini file suggests that that indeed
> is the case. Some of these nice .ini comments should perhaps also be put
> here and explained in more detail here.)
>
>> +Error e-mails
>> +-------------
>> +
>> +When an exception occurs in Kallithea -- and unless interactive debugging
>> is
>> +enabled using ``set debug = true`` in the ``[app:main]`` section of the
>> +configuration file -- an e-mail with exception details is sent by
>> WebError_'s
>> +``ErrorMiddleware`` to the addresses specified in ``email_to`` in the
>> +configuration file.
>> +
>> +The ``From`` header of these e-mails is set using the
>> ``error_email_from``
>> +setting in the configuration file. The ``Subject`` of these e-mails can
>> +optionally be prefixed with the value of ``error_subject_prefix`` in the
>> +configuration file. This value defaults to '`WebApp Error:` '.
>> +
>> +_Note:_ Due to the way configuration files are parsed, trailing
>> whitespace is
>> +ignored. For ``error_subject_prefix``, however, a trailing space can be
>> +desirable. A simple solution is to add a special Unicode space instead,
>> for
>> +example `U+2002 (EN SPACE)`: ' '.
>
>
> Yeah, that parser seems to be very not-so-clever.
>
> Unicode in mail subjects is however a flaky thing (AFAIK). That workaround
> doesn't sound like something I would recommend ... Is that recommended from
> WebError upstream?

This was a suggestion from one of my creative brain cells... ;-)

>
> It sounds like it would be better to make a hack that checks if the
> configured value both start and ends with ", it will strip these and
> preserve whitespace inside it and write it back to the ini file data
> structure.

Such a change would need to be made in WebError, unless you want to
make such a hack from config/environment.py or something.

>
>> +_Note:_ The WebError_ package does not respect ``smtp_port`` and assumes
>> the
>> +standard SMTP port (25). If you have a remote SMTP server with a
>> different port,
>> +you could set up a local forwarding SMTP server on port 25.
>
>
> There has not been any other mentioning of smtp_port in this doc. Which part
> _is_ using it?
>
> If this is a real problem, what is WebError upstream's take on it?

The Kallithea (application) e-mails are correctly respecting the port.
The problem is only there for error e-mails.

I think it's a real problem, but maybe very few people will run into
it because most SMTP servers are simply run on port 25. I haven't
discussed it with WebError yet...

>
> This also suggests that it doesn't support smtp over ssl (which should be
> mandatory for all authorized delivery)? There has been some rumours of
> problems in this area. The lack of conclusion could be because different
> kind of mails have different capabilities?

If the rumours are always in regard to error e-mails, then it could be
related. For application e-mails, the port is respected so mails
should always arrive even over SSL. If not, it's a different problem
according to me.

>
>
> Fair enough if you consider some of these comments out of scope for now ...
> but please take another iteration while considering the comments here and
> perhaps add TO BE WRITTEN hints about stuff that has been left out for now.

I think I covered your comments now (v2 to be sent shortly), I left
out the error_subject_prefix for now.
Feel free to adapt further before merging these changes.

Thanks for the review,
Thomas