-
Notifications
You must be signed in to change notification settings - Fork 15
Introducing new handlers to maintain an rpm dep chain. #15
Introducing new handlers to maintain an rpm dep chain. #15
Conversation
|
||
# TODO - look for any further special psuedo-error handling cases here. | ||
import pprint | ||
pprint.pprint(body) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to keep this in for now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should probably change it to log.something(body)
.
I'd like to keep it because I'm not sure what kind of error PDC might throw back at me. If it is a new error I've never seen before, I'd like to know the details in the logs so I can make an appropriate patch to handle it in the future.
Ok, nothing widly standing out for me, but I must confess I'm not sure to follow everything and I might have been running low on coffee when I reviewed this one |
return ['buildsys.tag'] | ||
|
||
def can_handle(self, msg): | ||
if not msg['topic'].endswith('buildsys.tag'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if it'd be a good idea to use 'self.topic_suffixes' instead of hardcoding 'buildsys.tag'. Not a big deal for now, but as things grow, it might be nice to just update one location for what is supported in this handler.
pdcupdater.utils.ensure_release_exists(pdc, release_id, release) | ||
|
||
koji_relationships = self._yield_koji_relationships(pdc, tag) | ||
for parent, type, child in koji_relationships: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, can we rename type
to something like relationship_type
to not override the builtin type function.
@ralphbean looks good. I didn't look too closely at the tests though. Just a few comments to address and then go ahead and merge it. |
OK, after lots of refactoring, this is much faster. It used to take ~1 hour to process a single build. Now it takes ~1 minute. :) |
fd8a913
to
a136836
Compare
This is the first part of the puzzle for https://fedoraproject.org/wiki/User:Ralph/Drafts/Infrastructure/Factory2/ModellingDeps