I also have a question: Where do messages to the reply address go after the milter is done with them? Are they delivered to some other program for further processing? Archived in a mailbox? Or is Postfix configured to drop them?

I suspect the tests are failing for code-style reasons. If you run black and flake8 on your branch, it will most likely fix the tests.

Where do messages to the reply address go after the milter is done with them? Are they delivered to some other program for further processing? Archived in a mailbox? Or is Postfix configured to drop them?

In the case of pagure.io, postfix drops them

Knowing that messages aren't processed further after the milter is done with them, I have made some more changes. The milter now reports an error if a reply is neither a ticket reply nor a pull request reply. It also returns DROP when a message has been successfully delivered to the web server.

Knowing that messages aren't processed further after the milter is done with them, I have made some more changes.

I'm wondering about this, we drop them in the case of pagure.io, but can/should we assume that other pagure instance running the milter will do the same?

Pierre-YvesChibon wrote:

Knowing that messages aren't processed further after the milter is done=
with them, I have made some more changes. =20
=20
I'm wondering about this, we drop them in the case of pagure.io, but can/=
should we assume that other pagure instance running the milter will do the =
same?

I got the impression that the Pagure software has no further use for
them, so why keep them? Do you think that somebody might want to
archive them?

Anyway, I made separate commits precisely so you'd be able to pick the
ones you agree with.

pretty please pagure-ci rebuild

I don't understand. What is pagure-ci?

And I see that Jenkins continues to report "failure", but I can't find
a way to get a useful error message out of it.

Pierre-YvesChibon wrote:

Knowing that messages aren't processed further after the milter is done=
with them, I have made some more changes. =20
=20
I'm wondering about this, we drop them in the case of pagure.io, but can/=
should we assume that other pagure instance running the milter will do the =
same?

I got the impression that the Pagure software has no further use for
them, so why keep them? Do you think that somebody might want to
archive them?

That's what I was wondering, I don't think it's really worth it, but I was wondering if someone may think differently.

Anyway, I made separate commits precisely so you'd be able to pick the
ones you agree with.

pretty please pagure-ci rebuild

I don't understand. What is pagure-ci?

And I see that Jenkins continues to report "failure", but I can't find
a way to get a useful error message out of it.

That message triggers a run of the CI on this PR but the logs are not kept for long, so, we sometime have to re-trigger it to get "fresh" results again.

What I suspect seeing this in a different project, is that the latest version of black (https://pypi.org/project/black/ ) which we run in one of the environment is likely complaining about the code. I've triggered a new run to check this (I forgot to check the outcome of the tests yesterday after triggering the CI run).

Check the output, I can now see the message from black in the last test environment (which uses the latest black version):

 130 files would be reformatted, 69 files would be left unchanged.

Let's pin the black version to < 20.8b0 until we have it in a stable Fedora release.

Pierre-YvesChibon wrote:

Knowing that messages aren't processed further after the milter is done=
with them, I have made some more changes. =20
=20
I'm wondering about this, we drop them in the case of pagure.io, but can/=
should we assume that other pagure instance running the milter will do the =
same?

I got the impression that the Pagure software has no further use for
them, so why keep them? Do you think that somebody might want to
archive them?

That's what I was wondering, I don't think it's really worth it, but I was wondering if someone may think differently.

@ngompa @jlanda any thought on this question?

One possible solution would be to add a comment in the code saying something like: If you want the email to be processed by another milter after this one, change this line to ... and merge as is.

I think we can do that for now, and if someone wants to submit a PR to make this more interesting, they can.

:thumbsup:

Is this better then?

@rombobeorn perfect for me!

Do we want to clean up the history a bit?

Pierre-YvesChibon wrote:

Do we want to clean up the history a bit?

If "we" means me, then no, I don't feel a need to rewrite history. I
just want the milter to do the right thing.

This seems fine to merge as-is, even with the somewhat annoying "coding style" commit. It's reasonably partitioned.