GH-48623: [CI][Archery][Dev] Add missing headers to email reports

[kou]

Rationale for this change

Our email reports miss the following headers:

• MIME-Version: 1.0
• Content-Type: text/plain; charset="utf-8"
• Message-Id: ${AUTO_GENERATED_MESSAGE_ID}
• Date: ${DATE_IN_RFC_2822}

What changes are included in this PR?

Add these headers.

Are these changes tested?

Yes.

Are there any user-facing changes?

No.

• GitHub Issue: [CI][Archery][Dev] Add missing headers to email reports #48623

[github-actions]

⚠️ GitHub issue #48623 has been automatically assigned in GitHub to PR creator.

[sebbASF]

From #48623 (comment)

It looks as though all the headers can (and should) be populated in the ReportUtils.send_email method.

Could you explain the reason? If we use the approach, it's (a bit) difficult to implement dry-run. (We need to pass more arguments to ReportUtils.send_email.)

At present, the headers have to be generated in lots of places, which means more maintenance and more testing.

As to doing a dry-run, that depends on whether you need to see all the headers or just the body.

The Python email library builds up the message for sending, and you can just print the message instead of sending it.

[sebbASF]

AFAICT when you refer to dry-run, you mean using something like output.write(email_report.render("workflow_report"))

If the templates no longer contain the headers, then of course they won't be shown.

I think the simplest way round this is to add a includeHeadersparameter to JinjaReport.render, with default False. If True, the response should include the relevant header values.

The dry-run code would then need to be changed to

output.write(email_report.render("workflow_report"), True)

[kou]

Thanks. I understand your point. We should provide a function for general headers generation, right?

How about the 6b31c32 approach? EmailReport.render(template_name, headers) generates email.message.EmailMessage. It also fills the default header values such as Message-Id.

[sebbASF]

How about the 6b31c32 approach?

This does not solve the issue that mail headers need special handling, and still leaves multiple locations to be maintained and tested.

[kou]

This does not solve the issue that mail headers need special handling

What does the "special handing" mean? I thought that it's non ASCII characters encoding, folding a long line and so on. And I thought that all of them are processed by the email.message.EmailMessage.[] automatically.

and still leaves multiple locations to be maintained and tested.

What does the "multiple locations" mean? We should not have header related information in dev/archery/archery/ci/cli.py and dev/archery/archery/crossbow/cli.py, right?

[sebbASF]

What does the "special handing" mean?

I see that most of the header values are passed to EmailMessage[]
However, the code makes assumptions about the syntax of the From header.
You need to use https://docs.python.org/3/library/email.headerregistry.html#email.headerregistry.Address
It would be safer to do the same for the To: address as well.

Also I see that the tests don't account for sender_name.

What does the "multiple locations" mean?

That the same code appears in multiple locations.

By which I mean that the code transforms the data.
It would be better to pass just the raw values in the data, and have the render method do all the formatting etc.
This means dropping the Date header and leaving it for the render method to construct.