I already asked Mike if he can move this to https://pagure.io/taskotron/task-fedora-cloud-tests. But I didn't even know about the other one! :-) Mike, could you also move https://pagure.io/taskotron-upstream-atomic to https://pagure.io/taskotron/task-upstream-atomic (or task-atomic or task-fedora-atomic-tests or whatever)?

Also, maybe we could start synchronizing how we use git branches (always having develop and master, and using develop for dev and stg hosts and master for production hosts).

We already have compose type. @mkrizek, couldn't we use compose type everywhere here, and then schedule the right jobs with the right data (cloud jobs with data pointing to cloud images, atomic jobs with data pointing to atomic images, etc)? I don't have a complete idea how trigger works, pardon my ignorance.

It seems too many things are hardcoded here (path, filename, arch). Couldn't we get that info from the metadata?
https://kojipkgs.fedoraproject.org/compose/twoweek/Fedora-Atomic-25-20170302.0/compose/metadata/

@adamwill might have a good advice for us, I'm not familiar too much with compose metadata.

Why not have this as the very start of the method, rather than the end?

I'm actually returning the 'compose' type in the data for do_trigger(). Initially I thought I needed to have my own item_type - but I wasn't sure and wanted to get feedback. I feel like using 'compose' is fine and I can get rid of mentions of cloud_compose and atomic_compose.

The fedmsg doesn't provide that much data in and of itself, just status, what image it was (the compose_id, like "Fedora-Cloud-24-20170303.0") and the location of the directory where all the stuff was dumped. You can see an example here: https://apps.fedoraproject.org/datagrepper/id?id=2017-9346a5a8-31f8-4db2-9ee6-6080cb14ac6f&is_raw=true&size=extra-large

I just hard-coded the bits to get the qcow2 image out. AFAICT that directory structure isn't going to change, so I'm pretty comfortable relying on it. As for the arch, I think x86_64 is the only one we produce for cloud/atomic.

I'm just used to putting my exception handling at the end. Since we're just futzing with strings I don't imagine we'd see much of a performance improvement.

Yeah, I tried to suggest that trigger downloads the metadata file ($location/metadata/something.json) and parses out the path to the image it wants. That also gets rid of hardcoding architecture. It's not that hard (but please let's add a relatively quick timeout to the metadata download call). OTOH if this is a "we need to have something done ASAP" attempt, I don't really object too much in here. But in the long run, I think we'll need to do it the proper way, so at least a FIXME here would be nice.

OK, but you never know when someone adds something non-trivial to the middle, network queries or whatever (exactly as I suggested above). I'm generally trying to verify inputs and stop execution as soon as possible.

This is the kinda thing fedfind is for, yeah. File names and paths have changed before and could change again (they're talking about changing the Atomic image file names so they include 'AtomicHost' not just 'Atomic' right now, for instance):

https://pagure.io/pungi-fedora/pull-request/154
https://pagure.io/releng/issue/6413

I've been taking a stance that the metadata properties should be stickier than the file names; the 'AtomicHost' proposal also proposes changing the metadata 'subvariant' for the images, but I'm asking them not to do that.

You might be interested in the way we do this in the openQA scheduler, which is more or less shadowing the layout of the metadata and matching the dicts:

https://pagure.io/fedora-qa/fedora_openqa/blob/master/f/fedora_openqa/schedule.py#_59
https://pagure.io/fedora-qa/fedora_openqa/blob/master/f/fedora_openqa/config.py#_103

The openQA scheduler uses fedfind to do the work of downloading and parsing the images.json file (and for a few other helper functions): fedfind.release.get_release(location=location).all_images() gets you the list of image dicts from the compose at location, flattened a bit (I hate the unnecessary nesting by 'variant' and 'arch' in the original metadata, so the all_images version stuffs the variant and arch into each image dict and gives you a flat list). But downloading and parsing it yourself is pretty easy too, if you know the location of the compose.

Right now it's a "get this thing working" effort. I've added a FIXME.

I'm fine with that. I've moved it up in the latest diff.

er...

 - when:
     message_type: AtomicCompose
-    do:
-      - {discover: {repo: 'https://pagure.io/taskotron/task-upstream-atomic.git', branch="master"}}
-      - {discover: {repo: 'https://pagure.io/taskotron/task-fedora-cloud-tests.git', branch="master"}}
+  do:
+    - {tasks: [upstream-atomic, fedora-cloud-tests]}
 
 - when:
     message_type: CloudCompose
-    do:
-     - {discover: {repo: 'https://pagure.io/taskotron/task-fedora-cloud-tests.git', branch="master"}}
+  do:
+    - {tasks: [fedora-cloud-tests]}

I presume that it'll be able to find the tasks because of grokmirror? So grokmirror pulls all the taskotron/task-* repos and mirrors them so you can call them in this fashion? It's unclear to me how this actually functions.

Doesn't need to be, but it also doesn't hurt.