Package API regression about sos HOT 7 CLOSED

Let me start off by saying I'm not 100% up to speed with all of this issue.

Currently when a package is queried (via pkgByName(), allPkgsByName(), allPkgsByNameRegex()) the return value is a simple string (or generated array of strings) giving the package name and no other details.

I think augmenting the PackageManager classes to return the package version along with the name is a good idea. We might want to grow a new method for getting the version, or we might want to return something more structured from the pkg family of functions, though for now I like the idea of a new method.

For now I intend to leave the shell-out rpm -qa machinery in place but I'm seeing considerable runtime increases compared to sos-2.2's rpm-python based code so this may need to be revised as well at some point.

I don't see a problem with depending on the rpm python library for this plugin and not using it for the PackageManager. Plugins are designed to handle import failure much more gracefully than PackageManager classes.

from sos.

I think augmenting the PackageManager classes to return the package version
along with the name is a good idea. We might want to grow a new method for
getting the version, or we might want to return something more structured from
the pkg family of functions, though for now I like the idea of a new method.

Well, returning an object that had some fields (an RPM header object)
was the old behaviour - there's still code in the tree that expects this
and now doesn't work.

I'm not sure a new method is any cleaner. We still have to store the
information somewhere and I like the idea of a simple dictionary with
'version' etc. keys.

I'm not hugely attached to one or the other though but this would be in
keeping with past behaviour.

I don't see a problem with depending on the rpm python library for this plugin
and not using it for the PackageManager. Plugins are designed to handle import
failure much more gracefully than PackageManager classes.

Failing to import rpm-python on a Red Hat based distro is a catastrophic
failure - yum and many other system components would be broken so I'm
not sure this is a problem - I don't recall any reports where report
generation broke in earlier releases because of this.

That said the runtime inflation isn't that drastic in my current testing
so I'm not planning to change this immediately.

from sos.

I'm not married to growing a new method. My initial thought would be that it'd break fewer existing usages, but there may not be that many direct usages if most plugins use the packages member tuple. In any case I'm happy either way we go I think.

Failing to import rpm-python on a Red Hat based distro is a catastrophic
failure - yum and many other system components would be broken so I'm
not sure this is a problem - I don't recall any reports where report
generation broke in earlier releases because of this.

I'm actually thinking about trying to import rpm on a non-redhat-based distro would/could result in sosreport failure (or at least throw an Exception to deal with.) Importing a non-existing module in a plugin is the expected way to depend on that module just for that plugin and the importer framework handles that pretty well. I agree that if we don't have access to the rpm module on a redhat distro we are up a creek.

from sos.

I'm not married to growing a new method. My initial thought would be that it'd
break fewer existing usages, but there may not be that many direct usages if
most plugins use the packages member tuple. In any case I'm happy either way we
go I think.

Yeah we can just see which looks best when we get there.

I'm actually thinking about trying to import |rpm| on a non-redhat-based distro
would/could result in sosreport failure (or at least throw an Exception to deal
with.) Importing a non-existing module in a plugin is the expected way to depend
on that module just for that plugin and the importer framework handles that
pretty well. I agree that if we don't have access to the |rpm| module on a
redhat distro we are up a creek.

Gotcha - I see your concern. I guess we can look at wrapping that import
statement (as it's going to get loaded up for a check() anyway). I'll
try to do some benchmarks comparing the 2.2 and 2.3+ package list
loading to see if it's worth caring about.

from sos.

Actually maybe another thing to consider would be re-jigging startup somewhat so that we can print a banner and maybe even notify the user what we're doing - I noticed all of this because of the long pause before getting any output at the start of a run.

Dunno, maybe that's $crack but just a thought.

from sos.

Coming back to this I really don't think import failures in policy classes should be something we can't deal with. For non-Red Hat platforms we're not going to be using the policy anyway so an exception in check() (or while loading the policy) shouldn't matter.

from sos.

Per IRC conversation, closing this.

High-level item to review our PM abstraction as a whole at a later date however.

from sos.