launchpad-reviewers team mailing list archive

[Jürgen Gmach <mp+464936@xxxxxxxxxxxxxxxxxx>]

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.