Looking at the code in find_parent_images_with_package, this may need to be more complex, no?

Can we just abstract that snippet into its own method and use it in both places?

def find_parent_image_from_child(self, child_image, children):                                                                                                                                                     
    parent_brew_build = child_image.get("parent_brew_build")                                    
    if parent_brew_build:                                                                       
        return parent_brew_build                                                                
    # We need to resolve the image in here because "parent_image_builds" needs to be there      
    # and it gets populated when the image gets resolved.                                       
    child_image.resolve(self, children)                                                         
    # If the parent is not in `parent_brew_build` we can try to look for the parent in Brew,    
    # using the field `parent_image_builds` (searching for the nvr), which should always be there.
    # In case parent_brew_build is None and child_image["parent_image_builds"] == {},           
    # it means we found a base image and there's no parent image.                               
    if not parent_brew_build and child_image["parent_image_builds"]:                            
        parent_brew_build = [                                                                   
            i["nvr"] for i in child_image["parent_image_builds"].values()                       
            if i["id"] == child_image["parent_build_id"]][0]                                    
    return parent_brew_build                                                                    

Looking at the code in find_parent_images_with_package, this may need to be more complex, no?
Can we just abstract that snippet into its own method and use it in both places?
def find_parent_image_from_child(self, child_image, children):
parent_brew_build = child_image.get("parent_brew_build")
if parent_brew_build:
return parent_brew_build
# We need to resolve the image in here because "parent_image_builds" needs to be there
# and it gets populated when the image gets resolved.
child_image.resolve(self, children)
# If the parent is not in parent_brew_build we can try to look for the parent in Brew,
# using the field parent_image_builds (searching for the nvr), which should always be there.
# In case parent_brew_build is None and child_image["parent_image_builds"] == {},
# it means we found a base image and there's no parent image.
if not parent_brew_build and child_image["parent_image_builds"]:
parent_brew_build = [
i["nvr"] for i in child_image["parent_image_builds"].values()
if i["id"] == child_image["parent_build_id"]][0]

return parent_brew_build

And if you can't find a parent image, I wouldn't raise an error. Otherwise, it may happen for base images or images from scratch. It's probably safe to assume that if you can't find a parent image either in lightblue or based on Koji's build metadata, then there isn't one.

Sounds good, but why do you want to return return parent_brew_build right away if you find it? We should resolve the image first.

Sounds good, but why do you want to return return parent_brew_build right away if you find it? We should resolve the image first.

Oh! I didn't realize resolving was always needed. If it is, then just remove that return :)

Mmm but still we need to resolve the parent image, and not the child image.
But I get your point, we should not repeat the code. I can come up with the change and push it.

Oh Lui, I think I realized only now what you meant. I'll address the comments now.

I think we should remove this else-block entirely.

+ # it means we found a base image and there's no parent image.
+ if not parent_brew_build and child_image["parent_image_builds"]:

not parent_brew_build can be removed from this if.

+ child_image.resolve(self, children)

For find_parent_image_from_child, only need to resolve the image to get parent_image_builds from corresponding Brew build. ContainerImage.resolve resolves three kind of things and parent_image_builds is resolved in resolve_commit. Can we just make a call child_image.resolve_commit() here in order to save time from the other two "resolve" methods?

Suggest to write docstring for this method to describe the behaivor, input parameters and return value.

• child_image.resolve(self, children)

For find_parent_image_from_child, only need to resolve the image to get parent_image_builds from corresponding Brew build. ContainerImage.resolve resolves three kind of things and parent_image_builds is resolved in resolve_commit. Can we just make a call child_image.resolve_commit() here in order to save time from the other two "resolve" methods?

Good catch. We can get rid of the "children" parameter this way.

:thumbsup: LGTM!

Commit f860bfd8 fixes this pull-request