launchpad-reviewers team mailing list archive

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

If `run-one` will ever only be used by Launchpad itself, this might be a viable approach, but though, why can't we use `lpcraft run-one <job_name> <series> <arch>`? (or if preferred: `lpcraft run-one -j <job_name> -s <series> -a <arch>` or similar).

I'd like to discuss the proposed index based approach vs a "selector/name" based approach.



Diff comments:

> diff --git a/lpcraft/commands/run.py b/lpcraft/commands/run.py
> index 685ab70..6f2e8cc 100644
> --- a/lpcraft/commands/run.py
> +++ b/lpcraft/commands/run.py
> @@ -173,76 +243,27 @@ def run(args: Namespace) -> int:
>          if not jobs:
>              raise CommandError(f"No job definition for {job_name!r}")
>          for job in jobs:
> -            if host_architecture not in job.architectures:
> -                continue
> -            if job.run is None:
> -                raise CommandError(
> -                    f"Job {job_name!r} for {job.series}/{host_architecture} "
> -                    f"does not set 'run'"
> -                )
> +            _run_job(job_name, job, provider, getattr(args, "output", None))

In the long term we could use a configuration object, where all configuration gets merged into, ie from yaml, from cli, and from environment, so we only need to pass around this one object. -> `_run_job(job_name, configuration)`

>  
> -            remote_cwd = env.get_managed_environment_project_path()
> +    return 0
>  
> -            emit.progress(
> -                f"Launching environment for {job.series}/{host_architecture}"
> -            )
> -            with provider.launched_environment(
> -                project_name=cwd.name,
> -                project_path=cwd,
> -                series=job.series,
> -                architecture=host_architecture,
> -            ) as instance:
> -                if job.snaps:
> -                    for snap in job.snaps:
> -                        emit.progress(f"Running `snap install {snap}`")
> -                        install_from_store(
> -                            executor=instance,
> -                            snap_name=snap,
> -                            channel="stable",
> -                            classic=True,
> -                        )
> -                if job.packages:
> -                    packages_cmd = ["apt", "install", "-y"] + job.packages
> -                    emit.progress("Installing system packages")
> -                    with emit.open_stream(f"Running {packages_cmd}") as stream:
> -                        proc = instance.execute_run(
> -                            packages_cmd,
> -                            cwd=remote_cwd,
> -                            env=job.environment,
> -                            stdout=stream,
> -                            stderr=stream,
> -                        )
> -                run_cmd = ["bash", "--noprofile", "--norc", "-ec", job.run]
> -                emit.progress("Running the job")
> -                with emit.open_stream(f"Running {run_cmd}") as stream:
> -                    proc = instance.execute_run(
> -                        run_cmd,
> -                        cwd=remote_cwd,
> -                        env=job.environment,
> -                        stdout=stream,
> -                        stderr=stream,
> -                    )
> -                if proc.returncode != 0:
> -                    raise CommandError(
> -                        f"Job {job_name!r} for "
> -                        f"{job.series}/{host_architecture} failed with "
> -                        f"exit status {proc.returncode}.",
> -                        retcode=proc.returncode,
> -                    )
> -
> -                if (
> -                    job.output is not None
> -                    and getattr(args, "output", None) is not None
> -                ):
> -                    target_path = (
> -                        args.output / job_name / job.series / host_architecture
> -                    )
> -                    target_path.mkdir(parents=True, exist_ok=True)
> -                    _copy_output_paths(
> -                        job.output, remote_cwd, instance, target_path
> -                    )
> -                    _copy_output_properties(
> -                        job.output, remote_cwd, instance, target_path
> -                    )
> +
> +def run_one(args: Namespace) -> int:
> +    """Select and run a single job from a pipeline."""
> +    config = Config.load(Path(".launchpad.yaml"))
> +
> +    jobs = config.jobs.get(args.job, [])
> +    if not jobs:
> +        raise CommandError(f"No job definition for {args.job!r}")
> +    if args.index >= len(jobs):
> +        raise CommandError(
> +            f"No job definition with index {args.index} for {args.job!r}"
> +        )
> +    job = jobs[args.index]
> +
> +    provider = get_provider()
> +    provider.ensure_provider_is_available()
> +
> +    _run_job(args.job, job, provider, getattr(args, "output", None))
>  
>      return 0


-- 
https://code.launchpad.net/~cjwatson/lpcraft/+git/lpcraft/+merge/413081
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/lpcraft:run-one into lpcraft:main.