Add initial dependency loader (Hawkey based) #13
Conversation
lhh
force-pushed
the
master
branch
6 times, most recently
from
730e5ba
to
e29f59f
Compare
Signed-off-by: Lon Hohberger <[email protected]>
|
@lhh can you add that reference we talked about when you get a chance? I will go over this in depth on Monday, and I think that would be helpful to be able to look at |
|
Added. Within the code we specifically call things in package-py.cpp in the above repository which is extracted from repository information provided via libdnf. The bits that actually gather and parse repository metadata is in libdnf directly. |
|
@lhh We should update setup.py to include rpmreq as a requirement. No need to worry about any of the other requirements files, they are driven by setup.py. I will review the rest of the changes next week. |
|
Rpmreq is quite tied to Hawkey and does not have any CI integration tests
that run without Hawkey. That's why I stubbed it out lock, stock, and
barrel.
I'll fix this Monday morning.
…On Sat, Dec 1, 2018, 12:52 PM Jason Joyce ***@***.*** wrote:
@lhh <https://github.com/lhh> We should update setup.py to include rpmreq
as a requirement. No need to worry about any of the other requirements
files, they are driven by setup.py. I will review the rest of the changes
next week.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#13 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAqvNjPeJfoRs9puM10OA12g2L_SUgA9ks5u0sHvgaJpZM4Y6Q2j>
.
|
|
@lhh maybe a second pull request with that change isolated. That way if we find the change causes issues with testing on Travis we can decide what to do from there, but this pull request is not blocked. |
| """ | ||
|
|
||
| def __init__(self, **kwargs): | ||
| if 'config' not in kwargs: |
There was a problem hiding this comment.
What if we just collapse this down to a config param, and then add a helper method to parse this into whatever ConfigManager needs below to initialize it? Then we can have an explicit signature for init, which I think makes usage clearer (if would have 4 params: config, logger, filename, version, which could then be documented, so one know what to pass in).
| self.log = kwargs['logger'] | ||
| self.provided_log = True | ||
| else: | ||
| self.log = logging.getLogger(__name__) |
There was a problem hiding this comment.
This should use our logging helper of the same name, so the actual backend we log to can be specified. Hmm, actually, it should probably go one stepfuther. Shouldn't this just be in the config file,and then the Configmanager loads the proper logger defined there? In that casewedon't even need to take the logger here,just set it up in the manager
There was a problem hiding this comment.
Right, we can have a logging configuration snippet that then just wraps our existing logging infra - good idea.
So, we then don't need it as part of the init function at all, which is good.
| self.wrong_version = transmogrify_unmet(wrong_version) | ||
| self.unmet = transmogrify_unmet(unmet_deps) | ||
|
|
||
| if 'logger' in kwargs: |
There was a problem hiding this comment.
I would rather not see us interacting with args passed to the object on instantiation here, and simply always use self.logger.
| return str_format.format(fname, len(self.met), len(self.wrong_version), len(self.unmet)) | ||
|
|
||
|
|
||
| def transmogrify_met(met_deps): |
There was a problem hiding this comment.
I am not sure where to put this, so I am going to drop it here. I am proposing a slight change in the design of this, suggesting that DependencySet do more what it sounds like (to me, at least), and return a set of Dependency objects. I notice that met, wrong_version and un_met all return hashes with the same fields, so why not simply add a type field or similar to denote met, un_met, or wrong_version. Then your transmogrify methods change to simply populate the current object based on type, and the Set object calls in the init something like:
self.met = [[met_dep for Dependency(type='met', met_dep) in met_deps]]
self.wrong_version = [[wrong for Dependency(type='wrong_version', wrong) in wrong_version]]
self.unmet = [[unmet for Dependency(type='unmet', unmet) in unmet_deps]]
| return (epoch, version, release) | ||
|
|
||
|
|
||
| def dep_to_nevr(dep): |
There was a problem hiding this comment.
These could probably remain outside the object, making them more reusable (and possibly moving them to toolchest?)
There was a problem hiding this comment.
+1 to making a toolchest rpm module.
| - http://whatever-location-2/x86_64 | ||
| build-version-2: | ||
| - http://whatever-location-3/arch/ | ||
| """ |
There was a problem hiding this comment.
Thank you for the nice doc string here!
| return (epoch, version, release) | ||
|
|
||
|
|
||
| def dep_to_nevr(dep): |
There was a problem hiding this comment.
+1 to making a toolchest rpm module.
| return ret | ||
|
|
||
|
|
||
| def _main(argv): |
There was a problem hiding this comment.
At some point we should remove this. But that can be at a later time when we have more of the runner infrastructure setup. We could however pull this out to a contrib or manual test area that just calls this module.
I'm working on other bits as well for pungi integration, but, wanted to make sure I got the layouts right.
This uses Hawkey to interface with libdnf and pull/parse package and repository metadata:
https://github.com/rpm-software-management/libdnf/tree/master/python/hawkey