Parsing this hurts :).

Can you also add test to check that if you pass "openssl-1.2.3-2" (without "module+"), the image gets filtered out?

Parsing this hurts :).

LOL. Ok, I'll make it a bit more readable.

So, you want to include the image only in case the matching NVR in srpm_nvr and NVR in rpm["srpm_nevra"] both either:

a) contains module+ string.
b) does not contain module+ string.

This should be written like this: ("module+" in nvr1 and module+ in nvr2) or ("module+" not in nvr1 and "module+" not in nvr2).

@jkaluza can you check it again?

Is this any clearer?

Filter out container images which contain a component from a module but do not contain the module itself.

Can we log this? It looks like something that it would be useful during debugging.

Let's log here, too.

These two lines could be simplified as such:

for srpm_nvr in srpm_name_to_nvrs.get(rpm.get("srpm_name"), []): 

Should this be considered an error? Sounds more like an INFO-like level. The purpose of this method is to filter out images, it shouldn't be an error if it does exactly that.

Should this be considered an error? Sounds more like an INFO-like level. The purpose of this method is to filter out images, it shouldn't be an error if it does exactly that.

I've put error because the other "filter_out" function similar to this one has "error". But I would change both. I'll do it, maybe another commit.

@lucarval I've made the suggested changes. Could you please check again?

:+1:

I was thinking we could deduplicate the code to get rpms from ContainerImage by adding it to new method ContainerImage.get_rpms() which would return None if RPM manifest is not there and rpm_manifest[rpms] otherwise.

With your code, there are already 3 places where we repeat this same longish block of code.

Done, I've added a new commit about it. @jkaluza could you please check again?

+1

One you fix that TODO message... :0

Oh man... I need something that forbids me to push stuff if there's a "TODO" :)

Hm, I've just noticed the ret. This seems strange to me.

I would rather remove ret completely and instead of return [] when the rpm_manifest cannot be found, you can return None.

And later use it like:

rpms = image.get_rpms(ret)
if rpms is None:
    ret.append(image)

The reason is that I think this is mixing plain get_rpms with filtering in single method.

@jkaluza, ok, done :)
Can you check one last time?

Commit cdccb2de fixes this pull-request