canonical-ubuntu-qa team mailing list archive

[Skia <mp+477729@xxxxxxxxxxxxxxxxxx>]

Just one small improvement to the code, but overall looks good! :-)

Diff comments:

> diff --git a/charms/focal/autopkgtest-web/webcontrol/update-github-jobs b/charms/focal/autopkgtest-web/webcontrol/update-github-jobs
> index a90a75e..a1053f9 100755
> --- a/charms/focal/autopkgtest-web/webcontrol/update-github-jobs
> +++ b/charms/focal/autopkgtest-web/webcontrol/update-github-jobs
> @@ -24,6 +25,30 @@ swift_container_cache = None
>  result_cache = {}
>  
>  
> +def logfile_confirms_job(swift_conn, container, logfile_obj, params):
> +    _, contents = swift_conn.get_object(container, logfile_obj)
> +    logfile = gzip.decompress(contents)
> +    logfile_lines = logfile.splitlines()
> +    logfile_iter = iter(logfile_lines)

It's a bit weird to use the iterator directly like that. This is a bit "low level", and not really useful here. I suggest a simpler construction, along the lines of the following:
```
logfile = gzip.decompress(contents).decode("utf-8")
command_line = None
for line in logfile.splitlines():
    if "command line: " in line:
        command_line = line
        break
if command_line is None:
    return False
```

> +    command_line = None
> +    try:
> +        while not command_line:
> +            line = next(logfile_iter).decode("utf-8")
> +            if "command line: " in line:
> +                command_line = line
> +    except StopIteration:
> +        return False
> +    logging.debug(f"Command line is:\n{command_line}")
> +
> +    env = params["env"]
> +    for key_equals_value in env:
> +        if key_equals_value not in command_line:
> +            logging.info(f"{logfile_obj} does not match {env}")
> +            return False
> +    logging.info(f"{logfile_obj} DOES match {env}")
> +    return True
> +
> +
>  def result_matches_job(swift_conn, container, result_obj, params):
>      global result_cache
>  


-- 
https://code.launchpad.net/~andersson123/autopkgtest-cloud/+git/autopkgtest-cloud/+merge/477729
Your team Canonical's Ubuntu QA is requested to review the proposed merge of ~andersson123/autopkgtest-cloud:ugj-fix into autopkgtest-cloud:master.