Would it be more readable if you replaced * with ? ? It looks more obvious to me.

Nitpick - add an empty line above? :-)

I don't really understand the purpose of this code. Do you intend to add test suite coverage before merging this, or keep this comment in and do it some other time?

Yaay, master of nitpicks! Absolutely though

Can you please explain where arches get used and how?

I guess. Incidentaly, in the way I wrote the regexp, it ends up doing the same, although it was not the initial intention. My original intent was '/^dist.rpmgrill(\.[^.]+)*/'.
Anyway, this is just an example config. I could absolutely change it, don't really care about it. Nice find, though!

This means the missing result fedmsg can either ask for a particular arch, or otherwise all supported arches are triggered, right? That's great.

Does this have any relevance to this patch? Koji has been added into pypi recently, but I don't know how well it works, have you tested it? In readme, we still require it to be installed from an rpm.

Overall looks good to me, once we clarify some details. I wonder, could you add a trivial document into docs/ describing how to retrigger a task? We could then send a link to Fedora Infra folks and ask them to implement it into Bodhi or Greenwave, and it would be helpful for them to see a required fedmsg structure as a reference. Or we can ask them to suggest modifications to the structure, if needed.

The 'problem' here is, that the original code (return '%s' % (val,)) worked fine for most of the stuff, but if you try applying that on [u"unicode"] for example, the output value is a string like this: '[u"unicode"]' which is, sadly, not a valid yaml list.
They also specifically call this out in the comments above the FIXME to some level.
This change makes sure that for the iterables, "%s" is called on each element (key, value..) to get rid of the potential u prefix.

This is important when we want to be able to do ${variable} replacements in the trigger template with lists. And although we already do that with the list of blacklisted packages for abicheck. We always only ever provided a list of non-unicode strings to the method, which gets "%s"-ed "properly".

I don't plan on writing a test for this before merge though, ATM.

Precisely

Well, I have a virtualenv that does not accept external libs (because of coverage/pytest issues). Tested it with koji from pypi, and it works OK, so I decided to keep it in. Can absolutely get rid of it, if you deem it better/cleaner. /me does not care that much.

If it worked for you, keep it in. Just perhaps just make it a separate commit, thanks.

Overall looks good to me ...

Nothing against it, but this is mostly a placeholder for the moment. I don't really care for the name of the topic, or precise structure of the fedmsg. It just needs to have the right fields, and then we'll change the consumer code based on that.

I'm not sure what the process behind proposing new fedmessages is, and whether it could be plain "taskotron.result_missing" style that anybody could just fire, or if each respective system (bodhi/greenwave) would have it's own topic, and we would have to consume those.

@ralph @pingou - would you be able to shine some light on this?

This whole patch is a working (and tested), but still in the PoC territory, IMO. When writing this, I expected changes once we hit the reality of the fedmessages being emitted, but all it needs (then) is a change of the one consumer file.

@kparal Makes sense?

arches or ${arches}?
arches is used in the trigger code to overload the config.valid_arches. Aka, if you wanted the rpmgrill task to be executed only on i386 you'd add the arches: "i386" parameter to the tasks dict.
ad ${arches} - all the ${} replacements, are populated from the data extracted from the received fedmsg (see ${repo}, ${distgit_branch} or ${critpath_pkgs} for other examples of the same concept).

The trick here (with TaskotronResultMissing type messages) is, that instead of hard-coding which arches are to be executed, you allow the fedmsg to contain an arch key-valu in their body to explicitly ask for a rerun of (say rpmlint) for that one arch (see the taskotron_result_missing_msg.py).

roger

I'm not sure what the process behind proposing new fedmessages is, and whether it could be plain "taskotron.result_missing" style that anybody could just fire, or if each respective system (bodhi/greenwave) would have it's own topic, and we would have to consume those.

We already have taskotron.result.new, so taskotron.results.missing seems like an obvious choice. This fedmsg can then be triggered by anyone on the bus, I believe.

This whole patch is a working (and tested), but still in the PoC territory, IMO. When writing this, I expected changes once we hit the reality of the fedmessages being emitted, but all it needs (then) is a change of the one consumer file.

We can either wait until the fedmsg structure is clarified with all the parties, or commit this, send it as an existing proposal, and make changes in future commits, if needed. I think the latter would be fine.

The idea is that RATS will be the tool sending these messages, so the message topic and structure are still to be defined, giving us quite some flexibility :)

@pingou - yeah. I guess I should have been clearer in what I ask, so:

Can any application on the bus send any fedmsg topic message, or is it just one app per topic?

thx!

Can any application on the bus send any fedmsg topic message, or is it just one app per topic?

Part of the topic is hardcoded, it's normally something like:
org.fedoraproject...... (the rest of the topic being free-form).

Where environment is dev|stg|prod and service the name of the app, so rats would send something like:
org.fedoraproject..rats...

Note that there is no technical reasons for this, it is all a standard we decided on in order to have some consistency between applications regarding messages.

Closed via https://pagure.io/taskotron/taskotron-trigger/c/2862aafd0676b275d939a3b5888e587d61ae17a3?branch=develop