Left a few very minor comments, looks good to me overall!
About the Enum classes within the SOSSRecord class:
I usually don't love nested classes, and perhaps eventually we will want to add
other sorts of non-SOSS records that will use the same sort of Priority,
Package, etc, and we might want to re-use them - if so leaving it in a common
place makes sense.
That being said, I really like how tidy this looks - it represents exactly a
SOSSRecord with all that's nested within it, and I don't need to go and find
the definition anything anywhere else. So IMO there is no need to
overcomplicate.
Also, this all seems to follow the pattern in UCT (but cleaner with the
dataclass, which is a nice touch IMO).
So I'm happy with this approach
Diff comments:
> diff --git a/lib/lp/bugs/scripts/soss/sossrecord.py
> b/lib/lp/bugs/scripts/soss/sossrecord.py
> new file mode 100644
> index 0000000..466606b
> --- /dev/null
> +++ b/lib/lp/bugs/scripts/soss/sossrecord.py
I would consider naming this files `models.py` to keep consistency with the uct
scripts and overall within the repo.
> @@ -0,0 +1,139 @@
> +# Copyright 2025 Canonical Ltd. This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +import re
> +from dataclasses import dataclass
> +from datetime import datetime
> +from enum import Enum
> +from typing import Any, Dict, List, Optional
> +
> +import yaml
I'd add a
```
__all__ = [
"SOSSRecord",
]
```
to keep consistency of what we do in other files.
> +
> +# From `soss-cve-tracker/git-hooks/check-cve-syntax`
> +VALID_CHANNEL_REGEX = re.compile(r"^(focal|jammy|noble):[^/]+/stable$")
> +
> +
> +@dataclass
> +class SOSSRecord:
> +
> + class PriorityEnum(Enum):
> + NEEDS_TRIAGE = "Needs-triage"
> + NEGLIGIBLE = "Negligible"
> + LOW = "Low"
> + MEDIUM = "Medium"
> + HIGH = "High"
> + CRITICAL = "Critical"
> +
> + class PackageStatusEnum(Enum):
> + IGNORED = "ignored"
> + NEEDS_TRIAGE = "needs-triage"
> + RELEASED = "released"
> + NOT_AFFECTED = "not-affected"
> + DEFERRED = "deferred"
> + NEEDED = "needed"
> +
> + class PackageTypeEnum(Enum):
> + CONDA = "conda"
> + PYTHON = "python"
> + UNPACKAGED = "unpackaged"
> + MAVEN = "maven"
> + RUST = "rust"
> +
> + @dataclass
> + class Channel:
> + value: str
> +
> + def __post_init__(self):
> + if not VALID_CHANNEL_REGEX.match(self.value):
> + raise ValueError(f"Invalid channel format: {self.value}")
> +
> + @dataclass
> + class CVSS:
> + source: str
> + vector: str
> + base_score: float
> + base_severity: float
> +
> + BASE_SEVERITIES = {"LOW", "MEDIUM", "HIGH", "CRITICAL"}
> +
> + def __post_init__(self):
> + if not (0.0 <= self.base_score <= 10.0):
> + raise ValueError(f"Invalid base score: {self.base_score}")
> + if self.base_severity not in self.BASE_SEVERITIES:
> + raise ValueError(
> + f"Invalid base severity: {self.base_severity}"
> + )
> +
> + @dataclass
> + class Package:
> + name: str
> + channel: "SOSSRecord.Channel"
> + repositories: List[str]
> + status: "SOSSRecord.PackageStatusEnum"
> + note: str
> +
> + references: List[str]
> + notes: List[str]
> + priority: PriorityEnum
> + priority_reason: str
> + assigned_to: str
> + packages: Dict[PackageTypeEnum, List[Package]]
> + candidate: Optional[str] = None
> + description: Optional[str] = None
> + cvss: Optional[List[CVSS]] = None
> + public_date: Optional[datetime] = None
> +
> + @classmethod
> + def from_yaml(cls, yaml_str: str) -> "SOSSRecord":
> + raw: Dict[str, Any] = yaml.safe_load(yaml_str)
> + return cls.from_dict(raw)
> +
> + @classmethod
> + def from_dict(cls, raw: Dict[str, Any]) -> "SOSSRecord":
> + priority = SOSSRecord.PriorityEnum(raw["Priority"])
I see some fields use `get` and others (like Piority) don't. I'm guessing some
are mandatory and others can be empty or non existant?
> +
> + packages = {}
> + for enum_key, pkgs in raw.get("Packages", {}).items():
> + package_type = SOSSRecord.PackageTypeEnum(enum_key.lower())
> + package_list = [
> + SOSSRecord.Package(
> + name=package["Name"],
> + channel=SOSSRecord.Channel(package["Channel"]),
> + repositories=package["Repositories"],
> + status=SOSSRecord.PackageStatusEnum(
> + package["Status"].lower()
> + ),
> + note=package["Note"],
> + )
> + for package in pkgs
> + ]
> + packages[package_type] = package_list
> +
> + cvss_list = [
> + SOSSRecord.CVSS(
> + cvss["source"],
> + cvss["vector"],
> + cvss["baseScore"],
> + cvss["baseSeverity"],
> + )
> + for cvss in raw.get("CVSS", [])
> + ]
> +
> + public_date_str = raw.get("PublicDate")
> + public_date = (
> + datetime.fromisoformat(public_date_str)
> + if public_date_str
> + else None
> + )
> +
> + return cls(
> + references=raw.get("References", []),
> + notes=raw.get("Notes", []),
> + priority=priority,
> + priority_reason=raw.get("Priority-Reason", ""),
> + assigned_to=raw.get("Assigned-To", ""),
> + packages=packages,
> + candidate=raw.get("Candidate"),
> + description=raw.get("Description"),
> + cvss=cvss_list,
> + public_date=public_date,
> + )
--
https://code.launchpad.net/~enriqueesanchz/launchpad/+git/launchpad/+merge/487018
Your team Launchpad code reviewers is requested to review the proposed merge of
~enriqueesanchz/launchpad:soss-import-parsing into launchpad:master.
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
