launchpad-reviewers team mailing list archive

[Guruprasad <mp+433493@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/lpcraft/config.py b/lpcraft/config.py
> index d8d2045..f60cbc2 100644
> --- a/lpcraft/config.py
> +++ b/lpcraft/config.py
> @@ -119,6 +121,13 @@ class PackageSuite(str, Enum):
>      jammy = "jammy"  # 22.04
>  
>  
> +class PPAShortFormURL(pydantic.ConstrainedStr):
> +    """A string with a constrained syntax to match a PPA short form URL."""
> +
> +    strict = True
> +    regex = re.compile(r"^[a-z0-9][a-z0-9\+\._\-]+/[a-z0-9][a-z0-9\+\._\-]+$")

I checked the snapcraft parameter that Colin had referred to in the related bug report and there they do not use a duplicate `ppa` prefix causing the key-value pair to look like `ppa: ppa:launchpad/ppa`. So I updated the regex to drop the duplicate `ppa:` prefix but missed updating the example below. Will update it in the next revision while addressing review comments.

> +
> +
>  class PackageRepository(ModelConfigDefaults):
>      """A representation of a package repository.
>  
> @@ -126,12 +135,57 @@ class PackageRepository(ModelConfigDefaults):
>      """
>  
>      type: PackageType  # e.g. `apt``
> +    ppa: Optional[PPAShortFormURL]  # e.g. `ppa:launchpad/ppa`

Like mentioned in the reply to Andrey's comment above, value in this example needs to be updated to `launchpad/ppa` so that the key-value pair in the configuration looks like `ppa: launchpad/ppa`.

>      formats: List[PackageFormat]  # e.g. `[deb, deb-src]`
> -    components: List[PackageComponent]  # e.g. `[main, universe]`
> +    components: Optional[List[PackageComponent]]  # e.g. `[main, universe]`
>      suites: List[PackageSuite]  # e.g. `[bionic, focal]`
> -    url: AnyHttpUrl
> +    url: Optional[AnyHttpUrl]
>      trusted: Optional[bool]
>  
> +    @root_validator(pre=True)
> +    def validate_multiple_fields(
> +        cls, values: Dict[str, Any]
> +    ) -> Dict[str, Any]:
> +        if "url" not in values and "ppa" not in values:
> +            raise ValueError(
> +                "One of the following keys is required with an appropriate"
> +                " value: 'url', 'ppa'."
> +            )
> +        elif "url" in values and "ppa" in values:
> +            raise ValueError(
> +                "Only one of the following keys can be specified:"
> +                " 'url', 'ppa'."
> +            )
> +        elif "ppa" not in values and "components" not in values:
> +            raise ValueError(
> +                "One of the following keys is required with an appropriate"
> +                " value: 'components', 'ppa'."
> +            )
> +        elif "ppa" in values and "components" in values:
> +            raise ValueError(
> +                "The 'components' key is not allowed when the 'ppa' key is"
> +                " specified. PPAs only support the 'main' component."
> +            )
> +        return values
> +
> +    @validator("components", pre=True, always=True)
> +    def infer_components_if_ppa_is_set(
> +        cls, v: List[PackageComponent], values: Dict[str, Any]
> +    ) -> List[PackageComponent]:
> +        if v is None and values["ppa"]:
> +            return ["main"]
> +        return v
> +
> +    @validator("url", pre=True, always=True)
> +    def infer_url_if_ppa_is_set(
> +        cls, v: AnyHttpUrl, values: Dict[str, Any]
> +    ) -> AnyHttpUrl:
> +        if v is None and values["ppa"]:
> +            return "{}/{}/ubuntu".format(
> +                LAUNCHPAD_PPA_BASE_URL, values["ppa"].replace("ppa:", "")
> +            )
> +        return v
> +
>      @validator("trusted")
>      def convert_trusted(cls, v: bool) -> str:
>          # trusted is True or False, but we need `yes` or `no`


-- 
https://code.launchpad.net/~lgp171188/lpcraft/+git/lpcraft/+merge/433493
Your team Launchpad code reviewers is requested to review the proposed merge of ~lgp171188/lpcraft:easier-way-to-add-a-ppa into lpcraft:main.