[Merge] ~cpete/ubuntu/+source/apport:updated-subiquity-hook into ~ubuntu-core-dev/ubuntu/+source/apport:ubuntu/devel

[Benjamin Drung]

Review: Approve

Merged with the pylint, mypy, etc complains addressed (no need for doing another iteration of review).

Diff comments:

> diff --git a/debian/package-hooks/subiquity.py b/debian/package-hooks/subiquity.py
> index e69be79..b5d7a52 100644
> --- a/debian/package-hooks/subiquity.py
> +++ b/debian/package-hooks/subiquity.py
> @@ -1,28 +1,44 @@
>  """Send reports about subiquity to the correct Launchpad project."""
>  
>  import os
> +import re
>  
> -from apport import hookutils
> -
> -
> -def add_info(report, unused_ui):
> -    """Send reports about subiquity to the correct Launchpad project."""
> -    # TODO: read the version from the log file?
> -    logfile = os.path.realpath("/var/log/installer/subiquity-debug.log")
> -    revision = "unknown"
> -    if os.path.exists(logfile):
> -        hookutils.attach_file(report, "logfile", "InstallerLog")
> -        with open(logfile, encoding="utf-8") as log_fp:
> -            first_line = log_fp.readline()
> -        marker = "Starting Subiquity revision"
> -        if marker in first_line:
> -            revision = first_line.split(marker)[1].strip()
> -    report["Package"] = f"subiquity ({revision})"
> -    report["SourcePackage"] = "subiquity"
> +from apport import Report, hookutils
> +from apport.ui import HookUI
>  
> -    # Package and SourcePackage keys are deleted when the Snap is chosen,
> -    # so make a separate key to keep track of the revision
> -    report["SnapVersion"] = revision
> +
> +def get_journal(report: Report, log_map: dict[str, str]) -> None:
> +    """Get installer environment journal log.
> +
> +    Appends the installer specific journal to the log map if it exists,
> +    otherwise appends the system journal to the report directly.
> +    """
> +    installer_journal = "/var/log/installer/installer-journal.txt"
> +    if os.path.exists(installer_journal):
> +        log_map["InstallerJournal"] = installer_journal
> +    else:
> +        report["SystemJournal"] = hookutils.recent_syslog(re.compile("."))
> +
> +
> +def get_revision(debug_log_contents: str) -> str:
> +    """Get subiquity revision from debug log contents.
> +
> +    Returns revision found (e.g. "1234") or "unknown" if not found.
> +    """
> +

pydocstyle complains: D202: No blank lines allowed after function docstring (found 1)

> +    # Revision information is always in the first line of the log file
> +    first_line = debug_log_contents.splitlines()[0]
> +    marker = "Starting Subiquity server revision"
> +    if marker in first_line:
> +        revision = first_line.split(marker)[1].strip()
> +    else:
> +        revision = "unknown"
> +
> +    return revision
> +
> +
> +def add_info(report: Report, unused_ui: HookUI):

mypy complains: error: Function is missing a return type annotation  [no-untyped-def]

> +    """Package hook entry point."""
>  

pydocstyle complains: D202: No blank lines allowed after function docstring (found 1)

>      # Check if the snap was updated
>      # TODO: Add logic to support this outside of the live environment.
> @@ -45,36 +61,34 @@ def add_info(report, unused_ui):
>      # add in hardware information
>      hookutils.attach_hardware(report)
>  
> +    # This key is important for checking revision information later
> +    DEBUG_LOG_KEY = "InstallerServerLog"

Variable name "DEBUG_LOG_KEY" doesn't conform to snake_case naming style (invalid-name)
I changed it to lowercase. Alternative would be to move that variable out of the function and declare it at the top of the file. If you prefer that, please open a merge request.

> +
>      # static subiquity generated logs
>      log_map = {
> -        "InstallerServerLog": "/var/log/installer/subiquity-server-debug.log",
> +        DEBUG_LOG_KEY: "/var/log/installer/subiquity-server-debug.log",
>          "InstallerServerLogInfo": "/var/log/installer/subiquity-server-info.log",
>          "InstallerClientLog": "/var/log/installer/subiquity-client-debug.log",
>          "InstallerClientLogInfo": "/var/log/installer/subiquity-client-info.log",
>          "CurtinLog": "/var/log/installer/curtin-install.log",
>          "CurtinAptConfig": "var/log/installer/curtin-install/subiquity-curtin-apt.conf",
> -        "CurtinCurthooksConfig": (
> -            "/var/log/installer/curtin-install/subiquity-curthooks.conf"
> -        ),
> -        "CurtinExtractConfig": (
> -            "/var/log/installer/curtin-install/subiquity-extract.conf"
> -        ),
> -        "CurtinInitialConfig": (
> -            "/var/log/installer/curtin-install/subiquity-initial.conf"
> -        ),
> -        "CurtinPartitioningConfig": (
> -            "/var/log/installer/curtin-install/subiquity-partitioning.conf"
> -        ),
> +        "CurtinCurthooksConfig": "/var/log/installer/curtin-install/subiquity-curthooks.conf",

pylint complains: C0301: Line too long (94/88) (line-too-long)
That's why I wrapped those lines.

> +        "CurtinExtractConfig": "/var/log/installer/curtin-install/subiquity-extract.conf",
> +        "CurtinInitialConfig": "/var/log/installer/curtin-install/subiquity-initial.conf",
> +        "CurtinPartitioningConfig": "/var/log/installer/curtin-install/subiquity-partitioning.conf",
>          "ProbeData": "/var/log/installer/block/probe-data.json",
>          "Traceback": "/var/log/installer/subiquity-traceback.txt",
>          "NetplanInstallerConfig": "/etc/netplan/00-installer-config.yaml",
>          "NetplanSnapdConfig": "/etc/netplan/00-snapd-config.yaml",
>      }
>  
> +    # Add journal log
> +    get_journal(report, log_map)
> +
>      # Add ubuntu-desktop-bootstrap information if available
>      udb_log = os.path.realpath("/var/log/installer/ubuntu_bootstrap.log")
>      if os.path.exists(udb_log):
> -        log_map["BoostrapLog"] = udb_log
> +        log_map["BootstrapLog"] = udb_log
>          report.add_tags(["ubuntu-desktop-bootstrap"])
>          # check for udb snap version
>          udb_snap = os.path.realpath("/snap/ubuntu-desktop-bootstrap/current")


-- 
https://code.launchpad.net/~cpete/ubuntu/+source/apport/+git/apport/+merge/454563
Your team Ubuntu Core Development Team is subscribed to branch ~ubuntu-core-dev/ubuntu/+source/apport:ubuntu/devel.