#802 Send one fedmsg message per push instead of per commit
Merged by pingou. Opened by pingou.
rework_fedmsg  into  master

Download 802.patch
no initial comment

Pull-Request has been rebased

Pull-Request has been rebased

This might result in massive fedmsg messages, and will break the current fedmsg_meta package.

Maybe at least make sure you're backwards compatible with fedmsg_meta?

Already merged upstream: https://github.com/fedora-infra/fedmsg_meta_fedora_infrastructure/pull/362

As for the size of the message, that is true although I think it won't be as big as the first mdapi messages that caused problem earlier.

Right, but this will still break backwards compatibility with people that haven't updated fedmsg_meta yet.

That's true for every change of fedmsg message we do, hasn't been a blocker so far.

Note: it won't break backward compatibility, it'll break forward one :)

I thought we were trying to make sure that they're at least backwards compatible with older fedmsg_meta package versions?

And if we know that massive messages are a problem, we shouldn't do this.
It might be short in most cases, but what happens if someone pushes the entire kernel history to Pagure?

atm you get a flood a messages on IRC/FMN...

With this change you get 1 notification.

I thought we were trying to make sure that they're at least backwards compatible with older fedmsg_meta package versions?

fedmsg_meta will be able to convert both the old and the new format

fedmsg_meta will be able to convert both the old and the new format

Right, but the old fedmsg_meta (before the fix) will crash, since it doesn't check if it's there: commit = msg['msg']['commit']['rev'].

Well, we'll just have to give people some lead time. Meaning: we need a release of fedmsg_meta to be put out and deployed.. and then wait until those updates go to stable before we can deploy a release of pagure with this fix. No?

Pull-Request has been updated

Ok PR updated to make the message much smaller and of size independent from the number of commits pushed.

:+1: from me. Let's wait to hear from @puiterwijk since he had some concerns though.

Looks good to me. Do make sure there is some time between release of the fedmsg-meta version and this patch though :).

Thanks for the reviews!

Pull-Request has been merged by pingou

Metadata