Skip to content

Conversation

@kou
Copy link
Member

@kou kou commented Dec 22, 2025

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-actions
Copy link

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

@github-actions github-actions bot added the awaiting committer review Awaiting committer review label Dec 22, 2025
@kou kou force-pushed the archery-ci-report-email-headers branch 5 times, most recently from 556d06f to 12b1981 Compare December 22, 2025 22:24
@kou kou force-pushed the archery-ci-report-email-headers branch from 12b1981 to 8cedff5 Compare December 22, 2025 23:56
@sebbASF
Copy link

sebbASF commented Dec 31, 2025

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
Copy link

sebbASF commented Dec 31, 2025

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
Copy link
Member Author

kou commented Jan 1, 2026

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting committer review Awaiting committer review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants