sts-sponsors team mailing list archive

[Björn Tillenius <mp+442831@xxxxxxxxxxxxxxxxxx>]

Review: Approve

+1 with a comment inline.

Diff comments:

> diff --git a/maaskpi/dailystats.py b/maaskpi/dailystats.py
> index e2bdab2..8c942c5 100644
> --- a/maaskpi/dailystats.py
> +++ b/maaskpi/dailystats.py
> @@ -185,68 +190,127 @@ def get_date(name):
>      return datetime.strptime(date_string, "%Y%m%d").date()
>  
>  
> -def normalize_data_dict(normalized):
> -    """Normalize the data dict to make it easier to work with.
> +def get_entry_stats(data) -> DeploymentStats:
> +    """Return stats for an entry."""
>  
> -    Older versions of MAAS might not have all the data, or have keys
> -    with different names. This function adds the missing keys and
> -    renames the old keys, so that the callsites can work with the latest
> -    version of the data dict schema.
> -    """
> -    if "workload_annotations" not in normalized:
> -        normalized["workload_annotations"] = {
> -            "annotated_machines": 0,
> -            "total_annotations": 0,
> -            "unique_keys": 0,
> -            "unique_values": 0,
> -        }
> -    if "vm_hosts" not in normalized:
> -        normalized["vm_hosts"] = {}
> -        for vm_type in ["lxd", "virsh"]:
> -            normalized["vm_hosts"][vm_type] = {
> -                "vm_hosts": 0,
> -                "vms": 0,
> -                "available_resources": {
> -                    "cores": 0,
> -                    "memory": 0,
> -                    "local_storage": 0,
> -                    "over_cores": 0,
> -                    "over_memory": 0,
> -                },
> -                "utilized_resources": {
> -                    "cores": 0,
> -                    "memory": 0,
> -                    "storage": 0,
> -                },
> -            }
> -    if "network_stats" not in normalized:
> -        normalized["network_stats"] = {
> -            "spaces": 0,
> -            "fabrics": 0,
> -            "vlans": 0,
> -            "subnets_v4": 0,
> -            "subnets_v6": 0,
> -        }
> -    if "bmcs" not in normalized:
> -        normalized["bmcs"] = {
> -            "auto_detected": {},
> -            "user_created": {},
> -            "unknown": {},
> -        }
> -    normalized["vm_hosts"]["lxd"]["available_resources"]["over_cores"] = int(
> -        normalized["vm_hosts"]["lxd"]["available_resources"]["over_cores"]
> -    )
> -    normalized["vm_hosts"]["virsh"]["available_resources"]["over_cores"] = int(
> -        normalized["vm_hosts"]["virsh"]["available_resources"]["over_cores"]
> -    )
> -    normalized["vm_hosts"]["lxd"]["available_resources"]["over_memory"] = int(
> -        normalized["vm_hosts"]["lxd"]["available_resources"]["over_memory"]
> -    )
> -    normalized["vm_hosts"]["virsh"]["available_resources"]["over_memory"] = int(
> -        normalized["vm_hosts"]["virsh"]["available_resources"]["over_memory"]
> +    def get(*keys: str) -> int:

Could you add a docstring explaining why this function is needed?

> +        stats = data
> +        for key in keys:
> +            try:
> +                stats = stats[key]
> +            except (KeyError, TypeError):
> +                # either the key is not found, or data is not a dict
> +                return 0
> +        try:
> +            return int(stats)
> +        except TypeError:
> +            return 0
> +
> +    return DeploymentStats(
> +        machines=get("nodes", "machines"),
> +        devices=get("nodes", "devices"),
> +        regionrack_controllers=get("controllers", "regionracks"),
> +        region_controllers=get("controllers", "regions"),
> +        rack_controllers=get("controllers", "racks"),
> +        vm_hosts_lxd_total=get("vm_hosts", "lxd", "vm_hosts"),
> +        vm_hosts_virsh_total=get("vm_hosts", "virsh", "vm_hosts"),
> +        vm_hosts_lxd_vms=get("vm_hosts", "lxd", "vms"),
> +        vm_hosts_virsh_vms=get("vm_hosts", "virsh", "vms"),
> +        vm_hosts_lxd_available_cores=get(
> +            "vm_hosts", "lxd", "available_resources", "cores"
> +        ),
> +        vm_hosts_virsh_available_cores=get(
> +            "vm_hosts",
> +            "virsh",
> +            "available_resources",
> +            "cores",
> +        ),
> +        vm_hosts_lxd_available_over_cores=get(
> +            "vm_hosts",
> +            "lxd",
> +            "available_resources",
> +            "over_cores",
> +        ),
> +        vm_hosts_virsh_available_over_cores=get(
> +            "vm_hosts",
> +            "virsh",
> +            "available_resources",
> +            "over_cores",
> +        ),
> +        vm_hosts_lxd_utilized_cores=get(
> +            "vm_hosts",
> +            "lxd",
> +            "utilized_resources",
> +            "cores",
> +        ),
> +        vm_hosts_virsh_utilized_cores=get(
> +            "vm_hosts", "virsh", "utilized_resources", "cores"
> +        ),
> +        vm_hosts_lxd_available_memory=get(
> +            "vm_hosts",
> +            "lxd",
> +            "available_resources",
> +            "memory",
> +        ),
> +        vm_hosts_virsh_available_memory=get(
> +            "vm_hosts",
> +            "virsh",
> +            "available_resources",
> +            "memory",
> +        ),
> +        vm_hosts_lxd_available_over_memory=get(
> +            "vm_hosts",
> +            "lxd",
> +            "available_resources",
> +            "over_memory",
> +        ),
> +        vm_hosts_virsh_available_over_memory=get(
> +            "vm_hosts",
> +            "virsh",
> +            "available_resources",
> +            "over_memory",
> +        ),
> +        vm_hosts_lxd_utilized_memory=get(
> +            "vm_hosts",
> +            "lxd",
> +            "utilized_resources",
> +            "memory",
> +        ),
> +        vm_hosts_virsh_utilized_memory=get(
> +            "vm_hosts",
> +            "virsh",
> +            "utilized_resources",
> +            "memory",
> +        ),
> +        workload_annotations_machines=get(
> +            "workload_annotations",
> +            "annotated_machines",
> +        ),
> +        workload_annotations_total=get(
> +            "workload_annotations",
> +            "total_annotations",
> +        ),
> +        workload_annotations_unique_keys=get(
> +            "workload_annotations",
> +            "unique_keys",
> +        ),
> +        workload_annotations_unique_values=get(
> +            "workload_annotations",
> +            "unique_values",
> +        ),
> +        subnets_v4=get("network_stats", "subnets_v4"),
> +        subnets_v6=get("network_stats", "subnets_v6"),
>      )
>  
> -    return normalized
> +
> +def get_bmc_stats(data):
> +    """Return and normalize bmc stats."""
> +    default = {
> +        "auto_detected": {},
> +        "user_created": {},
> +        "unknown": {},
> +    }
> +    return data.get("bmcs", default)
>  
>  
>  class DailyStats:


-- 
https://code.launchpad.net/~ack/maas-kpi/+git/maas-kpi/+merge/442831
Your team MAAS Committers is subscribed to branch maas-kpi:master.