Hi Mads, On Fri, Jun 12, 2015 at 11:41 PM, Mads Kiilerich <[email protected]> wrote: > On 06/12/2015 09:09 PM, Thomas De Schampheleire wrote: >> >> # HG changeset patch >> # User Cedric De Herdt <[email protected]> >> # Date 1434135934 -7200 >> # Fri Jun 12 21:05:34 2015 +0200 >> # Node ID c9c5310da1771baed04e70fb45293b8d212e8bb9 >> # Parent 42feaacb78feecb2a07a3ca19db6cf7ace3fd4c1 >> notification: use Sender and From header to clarify comment and pull >> request mails >> >> Current e-mails are sent from the Kallithea-configured e-mail address. The >> subject line then needs to refer to the user to be useful. >> Instead, use the author of comments and pull requests as 'From', and make >> the Kallithea-configured address the 'Sender' in accordance with RFC5322. > > > So this will not cause problems for domains using domain keys or other anti > spam "authenticated sender" schemes? > >> Additionally, add the changeset/pullrequest description in the subject. > > > Separate patch, please ... especially because these two aspects are > unrelated and have different concerns and review comments. > > >> [Thomas De Schampheleire: >> - extend commit message >> - update after threading requirements described in commit 8d45a14d3191 >> ] >> >> diff --git a/kallithea/lib/celerylib/tasks.py >> b/kallithea/lib/celerylib/tasks.py >> --- a/kallithea/lib/celerylib/tasks.py >> +++ b/kallithea/lib/celerylib/tasks.py >> @@ -247,7 +247,7 @@ def get_commits_stats(repo_name, ts_min_ >> @task(ignore_result=True) >> @dbsession >> -def send_email(recipients, subject, body='', html_body='', headers=None): >> +def send_email(recipients, subject, body='', html_body='', headers=None, >> author=None): >> """ >> Sends an email with defined parameters from the .ini files. >> @@ -274,7 +274,17 @@ def send_email(recipients, subject, body >> log.error("No recipients specified") >> return False >> - mail_from = email_config.get('app_email_from', 'Kallithea') >> + mail_sender = email_config.get('app_email_from', 'Kallithea') >> + if author: >> + # indicates that there is a real author which should be in the >> 'From' field >> + # the agent will be put in 'Sender' field: see >> http://tools.ietf.org/html/rfc5322#section-3.6.2 >> + mail_from = author >> + try: >> + headers['Sender'] = mail_sender >> + except TypeError: >> + headers = {'Sender': mail_sender} > > > That is for the case where headers is None? Then please detect that > explicitly with "headers is None" or "not headers". > > Or (assuming we have the default headers=None to avoid having mutable > objects as default values), have an early "it headers is None: headers={}" > in the function. > > Anyway, how about a first patch that refactors and consistently renames > "mail_from" to "envelope_from" (which seems to be the right technical term > for what it is), and then another patch for setting "header_from" (or reuse > mail_from).
After further investigation, I don't think that 'envelope_from' is the right term. The envelope is something _around_ the message, the message being the contents _and_ the headers. These headers include both From: and Sender: There exist indeed the possibility of having an envelope sender, but this is not what we're dealing with here. Some references: http://www.avolio.com/columns/E-mailheaders.html http://tools.ietf.org/html/rfc5322 /Thomas _______________________________________________ kallithea-general mailing list [email protected] http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
