Message schemas: improve the notifications

[abompard]

This PR contains 2 changes:

• set the one-line description to be the summary instead of the __str__ representation (which goes in the email body)
• set the chroot message severity to DEBUG, which will not generate a notification in FMN by default. Those are always paired with a build message, so it makes more sense to notify on that one.

Fixes: #3237

[xsuchy]

??? Why?

[abompard]

The chroot messages are always paired with a build message, which means two notifications for basically the same thing in FMN. By setting the chroot severity to DEBUG (which seems the least notify-worthy of the two), it will not generate a notification in FMN by default.

By setting the severity in a message, you can hint people who subscribe to your events which events are important, which are not, and which require more attention, pretty much like log entries.

Does this answer your question?

[FrostyX]

Does this answer your question?

It does for me but in that case, I would like to ask you if you could write something like this in a code comment because I'd probably just thought it was a typo when encountering it :-)

[FrostyX]

Thank you very much for the PR @abompard.

Fixes: #3237

If I understand correctly, it fixes the following problem mentioned in the issue comment
#3237 (comment)
but not the issue itself?

Meaning, that people won't get "duplicate" messages with missing information, which is an improvement, but we still won't send per-build messages correctly.

[abompard]

Right, it does not improve the build messages, so I guess we'll need another PR to fix that.
I'll add a comment.

[abompard]

Oh wait, the class I added the severity to is actually used for the copr.build.start messages as well! I got tricked by the name too ;-)

[abompard]

If I understand correctly, no message is triggered when a package's builds are done, right? Only when each build is done. Should I try to add that? Would you be open to a PR that does it?

[FrostyX]

If I understand correctly, no message is triggered when a package's builds are done, right? Only when each build is done. Should I try to add that? Would you be open to a PR that does it?

I think the situation is even worse. Or maybe we are just describing the same thing with different words.

Anyway, when a build is submitted for multiple chroots, when every chroot is started it sends both build.start and chroot.start and when every chroot ends, it sends build.end. There are many things wrong here.

1. When any of the chroots starts building, we should send chroot.start, that is fine. But we should send build.start only when the very first chroot from that build starts
2. We should not send build.end for every ended chroot, we should send chroot.end there
3. The build.end message should be sent only once when the last chroot from that build finishes

Should I try to add that? Would you be open to a PR that does it?

That would be very much appreciated @abompard :-)

[abompard]

Thanks for the explanation @FrostyX .

I've looked at the code, and what I understand is that all messages are sent by the backend, but I'm not sure the backend is aware of a Package Build being created, only of the "chroots" it has to build. I'm not sure it's aware of other chroots belonging to the same Build either.

I could make the frontend send the build.started (or build.created) message when a build is created, but at the moment the message bus abstraction is in copr-backend. I can move it to copr-common and instantiate it in copr-frontend when a package build is created (or rebuilt), and when it's done or cancelled for the build.ended message. But this is looking like a bigger change. Does it sound OK to you from a code structure perspective? Do you see a better solution?

[praiskup]

jakub

Anyway, when a build is submitted for multiple chroots, when every chroot is started it sends both build.start and chroot.start and when every chroot ends, it sends build.end. There are many things wrong here.

Agreement ❤️ I've been thinking many times about changing the status quo, but we should provide some "migration time" before we do a cut for the old message API. And I haven't found enough time to think this through.

But we should send build.start only when the very first chroot from that build starts

So this is IMO not possible short term. We should keep that old message as-is, and provide a new one aside.

@abompard

I could make the frontend send the build.started (or build.created) message when a build is created, but at the moment the message bus abstraction is in copr-backend.

While things are better nowadays, we still choke from time to time (the overall build is blocked on sending messages). We can not afford to send messages directly by web UI and but we need to send them async (and check it has been delivered).

That said; the easiest thing for external contributors would be to create new backend "action type", and let backend send the message.

[praiskup]

Some things that need modification is the backend side, frontend side and common.

[praiskup]

Would it be possible to keep __str__(), and add summary? It still seems like a convenient thing for experiments at least.

[abompard]

The __str__() function already defaults to something that can be used in experiments, I think :-)

[praiskup]

Ah, I already forgot the reasons for re-defining that, I think the multiline pretty prints just hurt my 👀 in the command line (though sub-class is not the place to fix this). Ok.

[praiskup]

Wait, per docs:

        .. note:: Sub-classes should override this method. It is used to create
            the body of email notifications and by other tools to display messages
            to humans.

That's why I probably opted-in originally to decide the preferred one-line format when doing print(msg).

[FrostyX]

We talked about this at a meeting and agreed that we like this PR as it is now and the improvements that I suggested should be done in a separate PR because it won't be a straightforward thing to do.

Thank you again for this PR @abompard and sorry it took so long to merge.