sts-sponsors team mailing list archive

[Adam Collard <mp+440659@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/src/maasserver/utils/osystems.py b/src/maasserver/utils/osystems.py
> index 9b09303..cb1e6aa 100644
> --- a/src/maasserver/utils/osystems.py
> +++ b/src/maasserver/utils/osystems.py
> @@ -403,6 +404,95 @@ def get_release(string):
>      return release
>  
>  
> +class InvalidSubarchKernelStringError(Exception):
> +    """Raised when subarch/kernel string does not match
> +    any of the supported formats"""
> +
> +
> +def parse_subarch_kernel_string(string):

try not to use stdlib module names for parameters - i suggest "kernel_string"

> +    """Extracts meaningful info from the 3.4-ish subarch kernel string
> +    with the following format:
> +    ```
> +        [platform-](ga|hwe[-edge])-(release)-[flavour-][edge]
> +    ```
> +
> +    This format matches all the older kernel subarches, but also
> +    allows having the platform specified. Examples of supported strings:
> +      - `hwe-x`
> +      - `ga-22.04`
> +      - `hwe-22.04-lowlatency`
> +      - `nvidia-ga-22.04`
> +      - `raspi-hwe-edge-22.04-lowlatency`
> +
> +    :param string: kernel subarch string
> +
> +    FIXME: This is a workaround for 3.4, it is not intended to become
> +      a long-term solution (unless we fail spectacularly with improved
> +      kernel model)

would be good to add documentation on the return type.

Which makes me think - I wonder if a namedtuple would be useful here? Feels easy to mix up the different variables given their similarity

> +    """
> +    # Kernel string is dash-separated in this format
> +    parts = string.split("-")
> +
> +    if len(parts) == 1:
> +        # Legacy format (v1 probably?): subarch is an actual subarch!
> +        # (now called platform because of "subarch" being abused)
> +        return "", "", string, ""
> +
> +    # Figure out the kernel channel
> +    channel_index = None
> +    channels_found = 0
> +    for possible_channel in ("hwe", "ga"):
> +        try:
> +            channel_index = parts.index(possible_channel)
> +            channels_found += 1
> +        except ValueError:

tighten this up around to cover just the parts.index and move the channels_found setting to the else

> +            # We expect ValueError
> +            pass
> +        else:
> +            channel = possible_channel
> +    if channels_found > 1:
> +        raise InvalidSubarchKernelStringError(
> +            f"Kernel {string} has multiple channels specified!"
> +        )
> +    if channel_index is None:
> +        # Once again, subarch is an actual subarch! We don't do that
> +        # anymore though.
> +        return "", "", string, ""
> +
> +    # Everything before channel is considered as platform
> +    platform = ""
> +    if channel_index > 0:
> +        platform = "-".join(parts[:channel_index])
> +        parts = parts[channel_index:]
> +
> +    # HWE channel could be also "hwe-edge", let's check if that is the case
> +    if channel == "hwe":
> +        if len(parts) > 1 and parts[1] == "edge":
> +            channel = "hwe-edge"
> +            # Get rid of that extra element so that release will be
> +            # at index 1
> +            parts = parts[1:]
> +        elif parts[-1] == "edge":
> +            channel = "hwe-edge"
> +            parts = parts[:-1]
> +
> +    # By this moment we should have release in parts[1] and flavour
> +    # as the rest of the parts.
> +    if len(parts) < 2:
> +        raise ValueError(f"Kernel {string} has no release specified")
> +    release = parts[1]
> +    kflavor = "-".join(parts[2:])
> +    return channel, release, platform, kflavor
> +
> +
> +# Coefficients for release sorting:
> +FLAVOURED_WEIGHT = 10
> +NOT_OLD_HWE_WEIGHT = 100  # New format kernels should weigh more
> +PLATFORM_WEIGHT = 1000
> +HWE_CHANNEL_WEIGHT = 10000
> +HWE_EDGE_CHANNEL_WEIGHT = 100000
> +
> +
>  def get_release_version_from_string(string):
>      """Convert an Ubuntu release, version, or kernel into a version tuple.
>  


-- 
https://code.launchpad.net/~igor-brovtsin/maas/+git/maas/+merge/440659
Your team MAAS Committers is subscribed to branch maas:master.