Add dist-git style tasks support
ClosedPublic

Authored by mkrizek on Apr 11 2016, 12:57 PM.

Details

Summary

This ended up being a bigger changed then I wanted to. The repo handling (making a shallow copy each time a fedmsg is received and deleting it afterwards) will probably change, but this seems like a good start for a testing run. Also, I noticed that replaying jobs from datagrepper behaves oddly, I'll look into it in another review.

Test Plan

Did some local testing with the following config option values:

[distgit]
cache_dir = /var/tmp/lib/taskotron-trigger
repo_url = https://bitbucket.org/
namespace = fedoraqa

and then I had to change the code so utils.get_repo gets 'fake-distgit' instead of a package name so I can test with fake dist-git repo. We'll do more testing once we have some 'guinea pigs' for actual dist-git tasks.

Worked for me, also unittests added/amended.

Diff Detail

Repository
rTRGR taskotron-trigger
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mkrizek retitled this revision from to Add dist-git style tasks support.Apr 11 2016, 12:57 PM
mkrizek updated this object.
mkrizek edited the test plan for this revision. (Show Details)
mkrizek added reviewers: tflink, kparal.
tflink requested changes to this revision.Apr 14 2016, 4:18 PM

Overall, looks pretty good.

I have a few small concerns and questions, though

conf/trigger.cfg.example
20

This needs to be http://pkgs.fedoraproject.org/git - you can't clone from the cgit endpoint

21

the namespace in which all the repos were created is rpms-checks

jobtriggers/config.py
22

same config value changes as noted above in the trigger.cfg.example

jobtriggers/koji_build_msg.py
36

What do we gain by making this so general? Is there a case where f isn't going to be JobTrigger.trigger_tasks?

jobtriggers/utils.py
40

Isn't this going to return 'f25' instead of 'master' for rawhide (with current definition of rawhide)?

This revision now requires changes to proceed.Apr 14 2016, 4:18 PM
mkrizek added inline comments.Apr 14 2016, 4:28 PM
conf/trigger.cfg.example
20

Will fix, thanks.

21

Will fix, thanks.

jobtriggers/koji_build_msg.py
36

Yes, it's needed in cli.py which uses its trigger_tasks method. This mess is all because of the cli where we can't use sanely methods from JobTrigger which depends on a fedmsg consumer (we use its logging methods) which cannot be sanely initiated without being plugged into fedmsg base consumer. I am open to suggestions though. :/ I suppose we can take out trigger_tasks from JobTrigger and pass logger - here we would pass fedmsg.logger and in cli cli.logger.

mkrizek added inline comments.Apr 14 2016, 4:31 PM
jobtriggers/utils.py
40

Good point. Any suggestions other than keeping the current master hardcoded (if 'f25' then 'master')?

tflink added inline comments.Apr 14 2016, 4:40 PM
jobtriggers/koji_build_msg.py
36

I don't have any better ideas, honestly. Just wasn't sure why it was needed

jobtriggers/utils.py
40

The only other thing I can think of is to default to master if the specified branch doesn't exist.

I suppose we could take yumrepoinfo out of libtaskotron and make it a standalone package but that sounds like overkill to me.

mkrizek updated this revision to Diff 2082.Apr 18 2016, 12:26 PM
  • Address issues in the review
tflink accepted this revision.Apr 20 2016, 6:12 PM

Looks good to me and works as expected when I run it locally.

I wonder if we should look at having a separate partition for the dist-git cache that we're kind of creating with this. Not really a "DO THIS NOW" kind of thing but I'd rather not have taskotron grind to a halt when we fill up / since this'll be running on the master :)

This revision is now accepted and ready to land.Apr 20 2016, 6:12 PM
In D808#15588, @tflink wrote:

Looks good to me and works as expected when I run it locally.

I wonder if we should look at having a separate partition for the dist-git cache that we're kind of creating with this. Not really a "DO THIS NOW" kind of thing but I'd rather not have taskotron grind to a halt when we fill up / since this'll be running on the master :)

Good point, I just now realized that we should maybe not create the cache for now and delete everything before each git pull. Right now we just delete the distgit cache of the koji build. So we don't have to watch just another dir that could eat the disk. :) Thoughts?

This revision was automatically updated to reflect the committed changes.