@pingou Please help to review my changes.

From a quick look this seems fine, I'll need to test it and we'll need to fix jenkins :)

@pingou
I test this function with the free online broker: iot.eclipse.org and it works.
If I want to add unit test or help to fix jenkins, what am I supposed to do ?

That sounds awesome .. I can give it a try on a centos mqtt broker soon on our test instance. Wondering about the "format" used in "message" : is that already a json payload ?
And what about the topic to publish to ? I see "topic" but it's not coming from pagure conf ?

@arrfab
I just wrote the function following the function format of the stomp, the topic and message should be a string.

Yes, but then how is topic defined and where ?
WRT the message payload itself, wondering if we wouldn't have to add an option to let user decide if he wants to receive json at the mqtt level, and so a simple json.dumps(message) .
But that assumes that message is actually a dict, which I admit I don't know, nor had a look at :)
I put this on my TOTEST list, but worth also adding eventually a TLS_CERT option also , so that pagure can use a TLS cert for auth, instead of just username/pass.

Let me also have a look at this

Yes, but then how is topic defined and where ?

Each message "type" has a different topic, they are the same as for fedmsg: https://fedora-fedmsg.readthedocs.io/en/latest/topics.html#id571

But that assumes that message is actually a dict, which I admit I don't know, nor had a look at :)

It's currently a dict indeed and if mqtt doesn't convert it to JSON then I think we should do it, +1

Ah, so something we can't configure then. So what about still having a MQTT_TOPIC variable in pagure.cfg that would let user create a parent topic in mqtt, and pagure would then be allowed in acl to write to all sub-topics .. example if parent topic in mqtt is set to git-prod, if someone pushes to pagure, the hooks would push to mqtt as /git-prod/pagure.git.receive (assuming that I understood the automatic topic, based on previous url for fedmsg)

Apart from the mqtt parent topic suggestion, it would be probably be good to have some other options, like for broker using TLS auth, so then client.tls_set(mqtt_cacert, tls_version=2) should be adapted.
https://www.eclipse.org/paho/clients/python/docs (and tls_set() has the doc for certfile,keyfile)

@jingjing are you still working on this PR? Did you see the suggestions from @arrfab ?

Hi @arrfab @pingou

Sorry that I missed the comment which arrfab added and have updated the PR according to comment, please help to review again, thank you!

Where is message being defined? In the function definition it's msg.

Also, is it a dict or JSON? I believe the function gets passed a dict and from the discussion with @arrfab I thought the idea was to convert it to JSON.

@jingjing did you see my last comment?

@jingjing ping?

@pingou
I've been busy recently, please help to review the updated PR.

@jingjing since you're busy, I'll take over the PR. To get jenkins to pass on this we need a few style changes.
Many thanks for your help on this, it's greatly appreciated, you did the lion's share of the work all I'll do is polishing :)

This is the follow up PR: https://pagure.io/pagure/pull-request/4132

@pingou Thank you. It's my pleasure working on this project:-)

Commit 4eda2c4d fixes this pull-request