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.
Details
- Reviewers
kparal tflink - Maniphest Tasks
- T749: Add support for dist-git style tasks for taskotron-trigger
- Commits
- rTRGR19d9f6a7f6c7: Add dist-git style tasks support
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.
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)? |
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. |
jobtriggers/utils.py | ||
---|---|---|
40 | Good point. Any suggestions other than keeping the current master hardcoded (if 'f25' then 'master')? |
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. |
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 needs to be http://pkgs.fedoraproject.org/git - you can't clone from the cgit endpoint