As mentioned last week, I lack some context to properly review this MP.

It is unclear, why images are missing for certain instances.

Also, the term "instances" was not used before in the script, but is now 
introduced. What is an "instance" in this context? It looks like is used 
interchangeably with application.

> This is important because we might have known issues with building some 
> images.

What kind of issues do we have? And why "might" and not always or never?


Diff comments:

> diff --git a/vbuilder/rebuild-images b/vbuilder/rebuild-images
> index b4f8348..b28fc02 100755
> --- a/vbuilder/rebuild-images
> +++ b/vbuilder/rebuild-images
> @@ -77,8 +78,17 @@ def main():
>              "juju", "ssh", unit, "sudo", 
> "/usr/local/bin/rebuild-latest-image",
>              f"{name_prefix}/ubuntu-{series}-daily-{arch}-",
>              ]
> -        utils.run(None, rebuild_cmd)
> +        try:
> +            utils.run(None, rebuild_cmd)
> +        except Exception as e:

Would it be possible to add a more specific exception?

> +            failed_target_instances.append(application)
> +            print(f"Command in `{application}` failed with error: {str(e)}")
>  
> +    print("\n\nRun completed.")
> +    if failed_target_instances:
> +        print("Rebuilding images failed in the following instances:")
> +        for instance in failed_target_instances:
> +            print(f" - {instance}")
>  
>  if __name__ == "__main__":
>      main()


-- 
https://code.launchpad.net/~ines-almeida/launchpad-mojo-specs/+git/private/+merge/464936
Your team Launchpad code reviewers is subscribed to branch 
~launchpad/launchpad-mojo-specs/+git/private:vbuilder.


_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to     : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Reply via email to